From 9fe2ca5dcf0da75f85d08930c2480d82b49ed1ae Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Wed, 5 Jul 2023 10:53:45 -0400 Subject: [PATCH] types/basetypes: Reduce memory allocations of (ObjectValue).ToTerraformValue() (#791) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/775 The prior implementation used the exported `AttributeTypes()` method, which in this case was unnecessarily creating a copy of the map when it was only be used for walking the existing data. At scale, such as handling 1000+ objects, this creates unnecessary pressure on the Go garbage collector. `benchstat` comparison with new benchmark test: ```text goos: darwin goarch: arm64 pkg: github.com/hashicorp/terraform-plugin-framework/types/basetypes │ original │ proposed │ │ sec/op │ sec/op vs base │ ObjectValueToTerraformValue1000-10 408.4µ ± 1% 318.4µ ± 1% -22.02% (p=0.000 n=10) │ original │ proposed │ │ B/op │ B/op vs base │ ObjectValueToTerraformValue1000-10 449.2Ki ± 0% 286.6Ki ± 0% -36.19% (p=0.000 n=10) │ original │ proposed │ │ allocs/op │ allocs/op vs base │ ObjectValueToTerraformValue1000-10 2.051k ± 0% 2.020k ± 0% -1.51% (p=0.000 n=10) ``` --- .../unreleased/BUG FIXES-20230703-123518.yaml | 6 ++++ types/basetypes/object_value.go | 6 ++-- types/basetypes/object_value_test.go | 30 +++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 .changes/unreleased/BUG FIXES-20230703-123518.yaml diff --git a/.changes/unreleased/BUG FIXES-20230703-123518.yaml b/.changes/unreleased/BUG FIXES-20230703-123518.yaml new file mode 100644 index 000000000..05e6c9cbe --- /dev/null +++ b/.changes/unreleased/BUG FIXES-20230703-123518.yaml @@ -0,0 +1,6 @@ +kind: BUG FIXES +body: 'types/basetypes: Minor reduction of memory allocations for `ObjectValue` type + `ToTerraformValue()` method, which decreases provider operation durations at scale' +time: 2023-07-03T12:35:18.484275-04:00 +custom: + Issue: "775" diff --git a/types/basetypes/object_value.go b/types/basetypes/object_value.go index 1a965dac0..baeb8c0eb 100644 --- a/types/basetypes/object_value.go +++ b/types/basetypes/object_value.go @@ -262,10 +262,12 @@ func (o ObjectValue) Type(ctx context.Context) attr.Type { // ToTerraformValue returns the data contained in the attr.Value as // a tftypes.Value. func (o ObjectValue) ToTerraformValue(ctx context.Context) (tftypes.Value, error) { - attrTypes := map[string]tftypes.Type{} - for attr, typ := range o.AttributeTypes(ctx) { + attrTypes := make(map[string]tftypes.Type, len(o.attributeTypes)) + + for attr, typ := range o.attributeTypes { attrTypes[attr] = typ.TerraformType(ctx) } + objectType := tftypes.Object{AttributeTypes: attrTypes} switch o.state { diff --git a/types/basetypes/object_value_test.go b/types/basetypes/object_value_test.go index cafc1fb3c..0801081d7 100644 --- a/types/basetypes/object_value_test.go +++ b/types/basetypes/object_value_test.go @@ -6,6 +6,7 @@ package basetypes import ( "context" "math/big" + "strconv" "testing" "github.com/google/go-cmp/cmp" @@ -15,6 +16,35 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" ) +func BenchmarkObjectValueToTerraformValue1000(b *testing.B) { + benchmarkObjectValueToTerraformValue(b, 1000) +} + +func benchmarkObjectValueToTerraformValue(b *testing.B, attributes int) { + attributeTypes := make(map[string]attr.Type, attributes) + attributeValues := make(map[string]attr.Value, attributes) + ctx := context.Background() + + for i := 0; i < attributes; i++ { + attributeName := "testattr" + strconv.Itoa(i) + attributeTypes[attributeName] = BoolType{} + attributeValues[attributeName] = NewBoolNull() + } + + value := NewObjectValueMust( + attributeTypes, + attributeValues, + ) + + for n := 0; n < b.N; n++ { + _, err := value.ToTerraformValue(ctx) + + if err != nil { + b.Fatalf("unexpected ToTerraformValue error: %s", err) + } + } +} + func TestNewObjectValue(t *testing.T) { t.Parallel()