tftypes: Remove Extraneous Memory Allocations in ApplyTerraform5AttributePathStep #307

bflad · 2023-06-29T12:19:56Z

terraform-plugin-go version

v0.17.0

Relevant provider source code

Code which generally calls tftypes.Walk()/tftypes.Transform() and involves collection/object types.

Terraform Configuration Files

Code with high numbers of elements/attributes.

resource "hashicups_order" "example2" {
  # items is a set of objects with additional attributes beyond quantity
  items = [
    for e in range(1, 1000) : {
      quantity = e
    }
  ]
}

Expected Behavior

Reasonable performance of tftypes.Walk() and tftypes.Transform() calls, let's arbitrarily say less than 1 second for handling a set of 1000 object elements.

Actual Behavior

Operations take many seconds with 1000 set of object elements, with a particular hot spot in (tftypes.Value).ApplyTerraform5AttributePathStep(). The code in that method is unnecessarily re-allocating an entire map/slice only to return the element at a map key or slice index. This causes many memory allocations on the heap and GC cycles.

Steps to Reproduce

Should be able to unit test benchmark this, but the real-world reproduction was done via terraform-provider-hashicups-pf.

References

Performance issue when run Terraform Plan with many Block Sets or NestedAttribute Sets terraform-plugin-framework#775

The text was updated successfully, but these errors were encountered:

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: ``` pkg: github.com/hashicorp/terraform-plugin-go/tftypes │ original.txt │ proposed.txt │ │ sec/op │ sec/op vs base │ ValueApplyTerraform5AttributePathStep1000-10 3838.5µ ± 1% 624.0µ ± 3% -83.74% (p=0.000 n=10) │ original.txt │ proposed.txt │ │ B/op │ B/op vs base │ ValueApplyTerraform5AttributePathStep1000-10 3887.0Ki ± 0% 389.3Ki ± 0% -89.98% (p=0.000 n=10) │ original.txt │ proposed.txt │ │ allocs/op │ allocs/op vs base │ ValueApplyTerraform5AttributePathStep1000-10 123.07k ± 0% 17.64k ± 0% -85.67% (p=0.000 n=10) pkg: github.com/hashicorp/terraform-plugin-go/tftypes │ transform-original.txt │ transform-proposed.txt │ │ sec/op │ sec/op vs base │ Transform1000-10 2876.6µ ± 1% 919.7µ ± 1% -68.03% (p=0.000 n=10) │ transform-original.txt │ transform-proposed.txt │ │ B/op │ B/op vs base │ Transform1000-10 3137.3Ki ± 0% 837.6Ki ± 0% -73.30% (p=0.000 n=10) │ transform-original.txt │ transform-proposed.txt │ │ allocs/op │ allocs/op vs base │ Transform1000-10 61.07k ± 0% 14.02k ± 0% -77.05% (p=0.000 n=10) ```

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% ```

…#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% ```

github-actions · 2024-05-23T05:06:45Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

bflad added the bug Something isn't working label Jun 29, 2023

bflad self-assigned this Jun 29, 2023

bflad mentioned this issue Jun 30, 2023

tftypes: Reduce AttributePath and Value type compute and memory usage #308

Merged

bflad closed this as completed in #308 Jul 3, 2023

github-actions bot locked as resolved and limited conversation to collaborators May 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tftypes: Remove Extraneous Memory Allocations in ApplyTerraform5AttributePathStep #307

tftypes: Remove Extraneous Memory Allocations in ApplyTerraform5AttributePathStep #307

bflad commented Jun 29, 2023 •

edited

Loading

github-actions bot commented May 23, 2024

tftypes: Remove Extraneous Memory Allocations in ApplyTerraform5AttributePathStep #307

tftypes: Remove Extraneous Memory Allocations in ApplyTerraform5AttributePathStep #307

Comments

bflad commented Jun 29, 2023 • edited Loading

terraform-plugin-go version

Relevant provider source code

Terraform Configuration Files

Expected Behavior

Actual Behavior

Steps to Reproduce

References

github-actions bot commented May 23, 2024

bflad commented Jun 29, 2023 •

edited

Loading