Merge pull request #26577 from hashicorp/jbardin/decoder-spec

Memoize Block.DecoderSpec

Author: jbardin

configs/configschema/decoder_spec.go

@@ -1,11 +1,74 @@
 package configschema
 
 import (
+	"runtime"
+	"sync"
+	"unsafe"
+
 	"github.com/hashicorp/hcl/v2/hcldec"
 )
 
 var mapLabelNames = []string{"key"}
 
+// specCache is a global cache of all the generated hcldec.Spec values for
+// Blocks. This cache is used by the Block.DecoderSpec method to memoize calls
+// and prevent unnecessary regeneration of the spec, especially when they are
+// large and deeply nested.
+// Caching these externally rather than within the struct is required because
+// Blocks are used by value and copied when working with NestedBlocks, and the
+// copying of the value prevents any safe synchronisation of the struct itself.
+//
+// While we are using the *Block pointer as the cache key, and the Block
+// contents are mutable, once a Block is created it is treated as immutable for
+// the duration of its life. Because a Block is a representation of a logical
+// schema, which cannot change while it's being used, any modifications to the
+// schema during execution would be an error.
+type specCache struct {
+	sync.Mutex
+	specs map[uintptr]hcldec.Spec
+}
+
+var decoderSpecCache = specCache{
+	specs: map[uintptr]hcldec.Spec{},
+}
+
+// get returns the Spec associated with eth given Block, or nil if non is
+// found.
+func (s *specCache) get(b *Block) hcldec.Spec {
+	s.Lock()
+	defer s.Unlock()
+	k := uintptr(unsafe.Pointer(b))
+	return s.specs[k]
+}
+
+// set stores the given Spec as being the result of b.DecoderSpec().
+func (s *specCache) set(b *Block, spec hcldec.Spec) {
+	s.Lock()
+	defer s.Unlock()
+
+	// the uintptr value gets us a unique identifier for each block, without
+	// tying this to the block value itself.
+	k := uintptr(unsafe.Pointer(b))
+	if _, ok := s.specs[k]; ok {
+		return
+	}
+
+	s.specs[k] = spec
+
+	// This must use a finalizer tied to the Block, otherwise we'll continue to
+	// build up Spec values as the Blocks are recycled.
+	runtime.SetFinalizer(b, s.delete)
+}
+
+// delete removes the spec associated with the given Block.
+func (s *specCache) delete(b *Block) {
+	s.Lock()
+	defer s.Unlock()
+
+	k := uintptr(unsafe.Pointer(b))
+	delete(s.specs, k)
+}
+
 // DecoderSpec returns a hcldec.Spec that can be used to decode a HCL Body
 // using the facilities in the hcldec package.
 //
@@ -18,6 +81,10 @@ func (b *Block) DecoderSpec() hcldec.Spec {
 		return ret
 	}
 
+	if spec := decoderSpecCache.get(b); spec != nil {
+		return spec
+	}
+
 	for name, attrS := range b.Attributes {
 		ret[name] = attrS.decoderSpec(name)
 	}
@@ -111,6 +178,7 @@ func (b *Block) DecoderSpec() hcldec.Spec {
 		}
 	}
 
+	decoderSpecCache.set(b, ret)
 	return ret
 }
 

lang/blocktoattr/fixup_bench_test.go

@@ -0,0 +1,97 @@
+package blocktoattr
+
+import (
+	"testing"
+
+	"github.com/hashicorp/hcl/v2"
+	"github.com/hashicorp/hcl/v2/hcldec"
+	"github.com/hashicorp/hcl/v2/hclsyntax"
+	"github.com/hashicorp/terraform/configs/configschema"
+	"github.com/zclconf/go-cty/cty"
+)
+
+func ambiguousNestedBlock(nesting int) *configschema.NestedBlock {
+	ret := &configschema.NestedBlock{
+		Nesting: configschema.NestingList,
+		Block: configschema.Block{
+			Attributes: map[string]*configschema.Attribute{
+				"a": {Type: cty.String, Required: true},
+				"b": {Type: cty.String, Optional: true},
+			},
+		},
+	}
+	if nesting > 0 {
+		ret.BlockTypes = map[string]*configschema.NestedBlock{
+			"nested0": ambiguousNestedBlock(nesting - 1),
+			"nested1": ambiguousNestedBlock(nesting - 1),
+			"nested2": ambiguousNestedBlock(nesting - 1),
+			"nested3": ambiguousNestedBlock(nesting - 1),
+			"nested4": ambiguousNestedBlock(nesting - 1),
+			"nested5": ambiguousNestedBlock(nesting - 1),
+			"nested6": ambiguousNestedBlock(nesting - 1),
+			"nested7": ambiguousNestedBlock(nesting - 1),
+			"nested8": ambiguousNestedBlock(nesting - 1),
+			"nested9": ambiguousNestedBlock(nesting - 1),
+		}
+	}
+	return ret
+}
+
+func schemaWithAmbiguousNestedBlock(nesting int) *configschema.Block {
+	return &configschema.Block{
+		BlockTypes: map[string]*configschema.NestedBlock{
+			"maybe_block": ambiguousNestedBlock(nesting),
+		},
+	}
+}
+
+const configForFixupBlockAttrsBenchmark = `
+maybe_block {
+  a = "hello"
+  b = "world"
+  nested0 {
+    a = "the"
+    nested1 {
+	  a = "deeper"
+      nested2 {
+        a = "we"
+        nested3 {
+          a = "go"
+          b = "inside"
+        }
+      }
+    }
+  }
+}
+`
+
+func configBodyForFixupBlockAttrsBenchmark() hcl.Body {
+	f, diags := hclsyntax.ParseConfig([]byte(configForFixupBlockAttrsBenchmark), "", hcl.Pos{Line: 1, Column: 1})
+	if diags.HasErrors() {
+		panic("test configuration is invalid")
+	}
+	return f.Body
+}
+
+func BenchmarkFixUpBlockAttrs(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		b.StopTimer()
+		body := configBodyForFixupBlockAttrsBenchmark()
+		schema := schemaWithAmbiguousNestedBlock(5)
+		b.StartTimer()
+
+		spec := schema.DecoderSpec()
+		fixedBody := FixUpBlockAttrs(body, schema)
+		val, diags := hcldec.Decode(fixedBody, spec, nil)
+		if diags.HasErrors() {
+			b.Fatal("diagnostics during decoding", diags)
+		}
+		if !val.Type().IsObjectType() {
+			b.Fatal("result is not an object")
+		}
+		blockVal := val.GetAttr("maybe_block")
+		if !blockVal.Type().IsListType() || blockVal.LengthInt() != 1 {
+			b.Fatal("result has wrong value for 'maybe_block'")
+		}
+	}
+}