Skip to content

Commit

Permalink
tftypes: Reduce AttributePath and Value type compute and memory usage (
Browse files Browse the repository at this point in the history
…#308)

Reference: #307

These optimizations were performed by adding benchmark tests against a set of 1000 "simple" objects and viewing the cpu/memory profiles. The original implementations were spending an immense amount of time dealing with memory allocations and garbage collection, so reducing memory allocations was the main target of these optimizations, which in turn, also reduced compute time.

The largest changes were accomplished by removing `(Value).Diff()` from the logic paths which were only testing equality. The new `(Value).deepEqual()` implementation started as a duplicate of that logic, removing all `ValueDiff` allocations. Then, the walking of `Value` needed a methodology to stop the walk immediately to prevent further allocations. The two changes in that regard were introducing a `stopWalkError` sentinel error type for which callback functions can signal that no remaining `Value` are necessary to traverse and updating the internal `walk()` implementation to include a new bool return to fully accomplish the intended behavior.

The other changes were smaller optimizations noticed from the profiling, such as avoiding the Go runtime immediately reallocating a larger slice with the `AttributePath` methods, avoiding memory allocations caused by `(Type).Is()` usage instead of using type switches, and directly referencing `List`, `Map`, `Object`, `Set`, and `Tuple` value storage instead of allocating an unneeded variable. This logic is heavily used both by this Go module and others, so even these minor optimizations have impact at scale.

`benchstat` differences between prior implementation and proposed changes:

```
goos: darwin
goarch: arm64
pkg: github.com/hashicorp/terraform-plugin-go/tftypes
                                             │   original   │              proposed               │
                                             │    sec/op    │   sec/op     vs base                │
Transform1000-10                               2886.1µ ± 0%   924.6µ ± 0%  -67.96% (p=0.000 n=10)
ValueApplyTerraform5AttributePathStep1000-10   3863.4µ ± 1%   628.9µ ± 0%  -83.72% (p=0.000 n=10)
geomean                                         3.339m        762.5µ       -77.16%

                                             │   original    │               proposed               │
                                             │     B/op      │     B/op      vs base                │
Transform1000-10                               3137.3Ki ± 0%   837.6Ki ± 0%  -73.30% (p=0.000 n=10)
ValueApplyTerraform5AttributePathStep1000-10   3887.1Ki ± 0%   389.3Ki ± 0%  -89.98% (p=0.000 n=10)
geomean                                         3.410Mi        571.0Ki       -83.65%

                                             │   original   │              proposed               │
                                             │  allocs/op   │  allocs/op   vs base                │
Transform1000-10                                61.07k ± 0%   14.02k ± 0%  -77.05% (p=0.000 n=10)
ValueApplyTerraform5AttributePathStep1000-10   123.07k ± 0%   17.64k ± 0%  -85.67% (p=0.000 n=10)
geomean                                         86.69k        15.72k       -81.86%
```
  • Loading branch information
bflad authored Jul 3, 2023
1 parent 653d9f3 commit e6a15f9
Show file tree
Hide file tree
Showing 12 changed files with 1,006 additions and 182 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/BUG FIXES-20230630-103655.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: BUG FIXES
body: 'tftypes: Significantly reduced compute and memory usage of `Value` type
walking and transformation'
time: 2023-06-30T10:36:55.119505-04:00
custom:
Issue: "307"
6 changes: 6 additions & 0 deletions .changes/unreleased/BUG FIXES-20230630-144701.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: BUG FIXES
body: 'tftypes: Removed unnecessary memory allocations from `AttributePath` type
`Equal()`, `LastStep()`, and `WithoutLastStep()` methods'
time: 2023-06-30T14:47:01.127283-04:00
custom:
Issue: "307"
5 changes: 5 additions & 0 deletions .changes/unreleased/BUG FIXES-20230630-144945.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
kind: BUG FIXES
body: 'tftypes: Removed unnecessary memory allocations from `NewValue()` function'
time: 2023-06-30T14:49:45.828209-04:00
custom:
Issue: "307"
6 changes: 6 additions & 0 deletions .changes/unreleased/ENHANCEMENTS-20230630-143817.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: ENHANCEMENTS
body: 'tftypes: Added `AttributePath` type `NextStep()` method, which returns the
next step in the path without first copying via `Steps()`'
time: 2023-06-30T14:38:17.600608-04:00
custom:
Issue: "307"
103 changes: 79 additions & 24 deletions tftypes/attribute_path.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,20 @@ func (a *AttributePath) String() string {
// AttributePaths are considered equal if they have the same number of steps,
// the steps are all the same types, and the steps have all the same values.
func (a *AttributePath) Equal(o *AttributePath) bool {
if len(a.Steps()) == 0 && len(o.Steps()) == 0 {
return true
if a == nil {
return o == nil || len(o.steps) == 0
}

if o == nil {
return len(a.steps) == 0
}
if len(a.Steps()) != len(o.Steps()) {

if len(a.steps) != len(o.steps) {
return false
}
for pos, aStep := range a.Steps() {
oStep := o.Steps()[pos]

for pos, aStep := range a.steps {
oStep := o.steps[pos]

if !aStep.Equal(oStep) {
return false
Expand Down Expand Up @@ -120,63 +126,112 @@ func (a *AttributePath) NewError(err error) error {
}
}

// LastStep returns the last step in the path. If the path was
// empty, nil is returned.
// LastStep returns the last step in the path. If the path is nil or empty, nil
// is returned.
func (a *AttributePath) LastStep() AttributePathStep {
steps := a.Steps()
if a == nil || len(a.steps) == 0 {
return nil
}

if len(steps) == 0 {
return a.steps[len(a.steps)-1]
}

// NextStep returns the next step in the path. If the path is nil or empty, nil
// is returned.
func (a *AttributePath) NextStep() AttributePathStep {
if a == nil || len(a.steps) == 0 {
return nil
}

return steps[len(steps)-1]
return a.steps[0]
}

// WithAttributeName adds an AttributeName step to `a`, using `name` as the
// attribute's name. `a` is copied, not modified.
func (a *AttributePath) WithAttributeName(name string) *AttributePath {
steps := a.Steps()
if a == nil {
return &AttributePath{
steps: []AttributePathStep{AttributeName(name)},
}
}

// Avoid re-allocating larger slice
steps := make([]AttributePathStep, len(a.steps)+1)
copy(steps, a.steps)
steps[len(steps)-1] = AttributeName(name)

return &AttributePath{
steps: append(steps, AttributeName(name)),
steps: steps,
}
}

// WithElementKeyString adds an ElementKeyString step to `a`, using `key` as
// the element's key. `a` is copied, not modified.
func (a *AttributePath) WithElementKeyString(key string) *AttributePath {
steps := a.Steps()
if a == nil {
return &AttributePath{
steps: []AttributePathStep{ElementKeyString(key)},
}
}

// Avoid re-allocating larger slice
steps := make([]AttributePathStep, len(a.steps)+1)
copy(steps, a.steps)
steps[len(steps)-1] = ElementKeyString(key)

return &AttributePath{
steps: append(steps, ElementKeyString(key)),
steps: steps,
}
}

// WithElementKeyInt adds an ElementKeyInt step to `a`, using `key` as the
// element's key. `a` is copied, not modified.
func (a *AttributePath) WithElementKeyInt(key int) *AttributePath {
steps := a.Steps()
if a == nil {
return &AttributePath{
steps: []AttributePathStep{ElementKeyInt(key)},
}
}

// Avoid re-allocating larger slice
steps := make([]AttributePathStep, len(a.steps)+1)
copy(steps, a.steps)
steps[len(steps)-1] = ElementKeyInt(key)

return &AttributePath{
steps: append(steps, ElementKeyInt(key)),
steps: steps,
}
}

// WithElementKeyValue adds an ElementKeyValue to `a`, using `key` as the
// element's key. `a` is copied, not modified.
func (a *AttributePath) WithElementKeyValue(key Value) *AttributePath {
steps := a.Steps()
if a == nil {
return &AttributePath{
steps: []AttributePathStep{ElementKeyValue(key)},
}
}

// Avoid re-allocating larger slice
steps := make([]AttributePathStep, len(a.steps)+1)
copy(steps, a.steps)
steps[len(steps)-1] = ElementKeyValue(key)

return &AttributePath{
steps: append(steps, ElementKeyValue(key.Copy())),
steps: steps,
}
}

// WithoutLastStep removes the last step, whatever kind of step it was, from
// `a`. `a` is copied, not modified.
func (a *AttributePath) WithoutLastStep() *AttributePath {
steps := a.Steps()
if len(steps) == 0 {
if a == nil || len(a.steps) == 0 {
return nil
}

return &AttributePath{
steps: steps[:len(steps)-1],
// Paths are immutable, so this should be safe without copying.
steps: a.steps[:len(a.steps)-1],
}
}

Expand Down Expand Up @@ -296,7 +351,7 @@ type AttributePathStepper interface {
// types need to use the AttributePathStepper interface to tell
// WalkAttributePath how to traverse themselves.
func WalkAttributePath(in interface{}, path *AttributePath) (interface{}, *AttributePath, error) {
if len(path.Steps()) < 1 {
if path == nil || len(path.steps) == 0 {
return in, path, nil
}
stepper, ok := in.(AttributePathStepper)
Expand All @@ -306,11 +361,11 @@ func WalkAttributePath(in interface{}, path *AttributePath) (interface{}, *Attri
return in, path, ErrNotAttributePathStepper
}
}
next, err := stepper.ApplyTerraform5AttributePathStep(path.Steps()[0])
next, err := stepper.ApplyTerraform5AttributePathStep(path.NextStep())
if err != nil {
return in, path, err
}
return WalkAttributePath(next, &AttributePath{steps: path.Steps()[1:]})
return WalkAttributePath(next, NewAttributePathWithSteps(path.steps[1:]))
}

func builtinAttributePathStepper(in interface{}) (AttributePathStepper, bool) {
Expand Down
Loading

0 comments on commit e6a15f9

Please sign in to comment.