-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tftypes: Reduce AttributePath and Value type compute and memory usage #308
Conversation
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 next biggest change was removing 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% ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Closes #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 allValueDiff
allocations. Then, the walking ofValue
needed a methodology to stop the walk immediately to prevent further allocations. The two changes in that regard were introducing astopWalkError
sentinel error type for which callback functions can signal that no remainingValue
are necessary to traverse and updating the internalwalk()
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 referencingList
,Map
,Object
,Set
, andTuple
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: