From 9b23badedd8f991a7a54c7a494d4ac110b39107e Mon Sep 17 00:00:00 2001 From: Pat Gavlin Date: Fri, 16 Jul 2021 09:56:26 -0700 Subject: [PATCH] [codegen/hcl2] Improve ConversionFrom perf. (#7545) - Lazily produce conversion failure diagnostics. This lowers the allocation volume and cuts down on execution time by avoiding the conversion of source and dest types to strings. - Add a fast path for union conversions that checks if the source type is identical to any of the union's element types. Type equality checks are generally much faster than type conversion checks. These changes lead to a significant speedup in codegen time in azure-native. --- pkg/codegen/hcl2/model/diagnostics.go | 3 ++- pkg/codegen/hcl2/model/type.go | 14 +++++++---- pkg/codegen/hcl2/model/type_const.go | 4 ++-- pkg/codegen/hcl2/model/type_eventuals.go | 10 ++++---- pkg/codegen/hcl2/model/type_list.go | 8 +++---- pkg/codegen/hcl2/model/type_map.go | 8 +++---- pkg/codegen/hcl2/model/type_none.go | 4 ++-- pkg/codegen/hcl2/model/type_object.go | 12 +++++----- pkg/codegen/hcl2/model/type_opaque.go | 6 ++--- pkg/codegen/hcl2/model/type_output.go | 4 ++-- pkg/codegen/hcl2/model/type_promise.go | 4 ++-- pkg/codegen/hcl2/model/type_set.go | 6 ++--- pkg/codegen/hcl2/model/type_tuple.go | 14 +++++------ pkg/codegen/hcl2/model/type_union.go | 30 ++++++++++++++++++------ 14 files changed, 74 insertions(+), 53 deletions(-) diff --git a/pkg/codegen/hcl2/model/diagnostics.go b/pkg/codegen/hcl2/model/diagnostics.go index 04704017e..bfb7fa825 100644 --- a/pkg/codegen/hcl2/model/diagnostics.go +++ b/pkg/codegen/hcl2/model/diagnostics.go @@ -35,7 +35,8 @@ func diagf(severity hcl.DiagnosticSeverity, subject hcl.Range, f string, args .. } func ExprNotConvertible(destType Type, expr Expression) *hcl.Diagnostic { - _, why := destType.conversionFrom(expr.Type(), false, map[Type]struct{}{}) + _, whyF := destType.conversionFrom(expr.Type(), false, map[Type]struct{}{}) + why := whyF() if len(why) != 0 { return errorf(expr.SyntaxNode().Range(), why[0].Summary) } diff --git a/pkg/codegen/hcl2/model/type.go b/pkg/codegen/hcl2/model/type.go index b7a8c222a..feabda8d4 100644 --- a/pkg/codegen/hcl2/model/type.go +++ b/pkg/codegen/hcl2/model/type.go @@ -19,6 +19,8 @@ import ( "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" ) +type lazyDiagnostics func() hcl.Diagnostics + type ConversionKind int const ( @@ -42,7 +44,7 @@ type Type interface { String() string equals(other Type, seen map[Type]struct{}) bool - conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, hcl.Diagnostics) + conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, lazyDiagnostics) string(seen map[Type]struct{}) string unify(other Type) (Type, ConversionKind) isType() @@ -74,14 +76,16 @@ func assignableFrom(dest, src Type, assignableFromImpl func() bool) bool { } func conversionFrom(dest, src Type, unifying bool, seen map[Type]struct{}, - conversionFromImpl func() (ConversionKind, hcl.Diagnostics)) (ConversionKind, hcl.Diagnostics) { + conversionFromImpl func() (ConversionKind, lazyDiagnostics)) (ConversionKind, lazyDiagnostics) { + if dest.Equals(src) || dest == DynamicType { return SafeConversion, nil } - if src, isUnion := src.(*UnionType); isUnion { + + switch src := src.(type) { + case *UnionType: return src.conversionTo(dest, unifying, seen) - } - if src, isConst := src.(*ConstType); isConst { + case *ConstType: return conversionFrom(dest, src.Type, unifying, seen, conversionFromImpl) } if src == DynamicType { diff --git a/pkg/codegen/hcl2/model/type_const.go b/pkg/codegen/hcl2/model/type_const.go index c2c12c911..a23b61904 100644 --- a/pkg/codegen/hcl2/model/type_const.go +++ b/pkg/codegen/hcl2/model/type_const.go @@ -74,8 +74,8 @@ func (t *ConstType) ConversionFrom(src Type) ConversionKind { return kind } -func (t *ConstType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, hcl.Diagnostics) { - return conversionFrom(t, src, unifying, seen, func() (ConversionKind, hcl.Diagnostics) { +func (t *ConstType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, lazyDiagnostics) { + return conversionFrom(t, src, unifying, seen, func() (ConversionKind, lazyDiagnostics) { if t.Type.ConversionFrom(src) != NoConversion { return UnsafeConversion, nil } diff --git a/pkg/codegen/hcl2/model/type_eventuals.go b/pkg/codegen/hcl2/model/type_eventuals.go index 4ef95feb8..c78a4ffda 100644 --- a/pkg/codegen/hcl2/model/type_eventuals.go +++ b/pkg/codegen/hcl2/model/type_eventuals.go @@ -69,7 +69,7 @@ func resolveEventualsImpl(t Type, resolveOutputs bool, seen map[Type]Type) (Type } elementTypes[i] = element } - return NewUnionType(elementTypes...), transform + return NewUnionTypeAnnotated(elementTypes, t.Annotations...), transform case *ObjectType: transform := makeIdentity if already, ok := seen[t]; ok { @@ -267,10 +267,6 @@ func inputTypeImpl(t Type, seen map[Type]Type) Type { return t } - if already, ok := seen[t]; ok { - return already - } - var src Type switch t := t.(type) { case *OutputType: @@ -288,6 +284,10 @@ func inputTypeImpl(t Type, seen map[Type]Type) Type { } src = NewUnionTypeAnnotated(elementTypes, t.Annotations...) case *ObjectType: + if already, ok := seen[t]; ok { + return already + } + properties := map[string]Type{} src = NewObjectType(properties, t.Annotations...) seen[t] = src diff --git a/pkg/codegen/hcl2/model/type_list.go b/pkg/codegen/hcl2/model/type_list.go index 6eb86cb55..fd66115f6 100644 --- a/pkg/codegen/hcl2/model/type_list.go +++ b/pkg/codegen/hcl2/model/type_list.go @@ -92,8 +92,8 @@ func (t *ListType) ConversionFrom(src Type) ConversionKind { return kind } -func (t *ListType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, hcl.Diagnostics) { - return conversionFrom(t, src, unifying, seen, func() (ConversionKind, hcl.Diagnostics) { +func (t *ListType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, lazyDiagnostics) { + return conversionFrom(t, src, unifying, seen, func() (ConversionKind, lazyDiagnostics) { switch src := src.(type) { case *ListType: return t.ElementType.conversionFrom(src.ElementType, unifying, seen) @@ -101,7 +101,7 @@ func (t *ListType) conversionFrom(src Type, unifying bool, seen map[Type]struct{ return t.ElementType.conversionFrom(src.ElementType, unifying, seen) case *TupleType: conversionKind := SafeConversion - var diags hcl.Diagnostics + var diags lazyDiagnostics for _, src := range src.ElementTypes { if ck, why := t.ElementType.conversionFrom(src, unifying, seen); ck < conversionKind { conversionKind, diags = ck, why @@ -112,7 +112,7 @@ func (t *ListType) conversionFrom(src Type, unifying bool, seen map[Type]struct{ } return conversionKind, diags } - return NoConversion, hcl.Diagnostics{typeNotConvertible(t, src)} + return NoConversion, func() hcl.Diagnostics { return hcl.Diagnostics{typeNotConvertible(t, src)} } }) } diff --git a/pkg/codegen/hcl2/model/type_map.go b/pkg/codegen/hcl2/model/type_map.go index b3e428172..666814c33 100644 --- a/pkg/codegen/hcl2/model/type_map.go +++ b/pkg/codegen/hcl2/model/type_map.go @@ -93,14 +93,14 @@ func (t *MapType) ConversionFrom(src Type) ConversionKind { return kind } -func (t *MapType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, hcl.Diagnostics) { - return conversionFrom(t, src, unifying, seen, func() (ConversionKind, hcl.Diagnostics) { +func (t *MapType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, lazyDiagnostics) { + return conversionFrom(t, src, unifying, seen, func() (ConversionKind, lazyDiagnostics) { switch src := src.(type) { case *MapType: return t.ElementType.conversionFrom(src.ElementType, unifying, seen) case *ObjectType: conversionKind := SafeConversion - var diags hcl.Diagnostics + var diags lazyDiagnostics for _, src := range src.Properties { if ck, _ := t.ElementType.conversionFrom(src, unifying, seen); ck < conversionKind { conversionKind = ck @@ -111,7 +111,7 @@ func (t *MapType) conversionFrom(src Type, unifying bool, seen map[Type]struct{} } return conversionKind, diags } - return NoConversion, hcl.Diagnostics{typeNotConvertible(t, src)} + return NoConversion, func() hcl.Diagnostics { return hcl.Diagnostics{typeNotConvertible(t, src)} } }) } diff --git a/pkg/codegen/hcl2/model/type_none.go b/pkg/codegen/hcl2/model/type_none.go index 0ef5939bf..52997860b 100644 --- a/pkg/codegen/hcl2/model/type_none.go +++ b/pkg/codegen/hcl2/model/type_none.go @@ -49,8 +49,8 @@ func (noneType) ConversionFrom(src Type) ConversionKind { return kind } -func (noneType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, hcl.Diagnostics) { - return conversionFrom(NoneType, src, unifying, seen, func() (ConversionKind, hcl.Diagnostics) { +func (noneType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, lazyDiagnostics) { + return conversionFrom(NoneType, src, unifying, seen, func() (ConversionKind, lazyDiagnostics) { return NoConversion, nil }) } diff --git a/pkg/codegen/hcl2/model/type_object.go b/pkg/codegen/hcl2/model/type_object.go index d37ff6690..e761bf1ad 100644 --- a/pkg/codegen/hcl2/model/type_object.go +++ b/pkg/codegen/hcl2/model/type_object.go @@ -183,13 +183,13 @@ func (t *ObjectType) ConversionFrom(src Type) ConversionKind { return kind } -func (t *ObjectType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, hcl.Diagnostics) { - return conversionFrom(t, src, unifying, seen, func() (ConversionKind, hcl.Diagnostics) { +func (t *ObjectType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, lazyDiagnostics) { + return conversionFrom(t, src, unifying, seen, func() (ConversionKind, lazyDiagnostics) { switch src := src.(type) { case *ObjectType: if seen != nil { if _, ok := seen[t]; ok { - return NoConversion, hcl.Diagnostics{invalidRecursiveType(t)} + return NoConversion, func() hcl.Diagnostics { return hcl.Diagnostics{invalidRecursiveType(t)} } } } else { seen = map[Type]struct{}{} @@ -205,7 +205,7 @@ func (t *ObjectType) conversionFrom(src Type, unifying bool, seen map[Type]struc } conversionKind := SafeConversion - var diags hcl.Diagnostics + var diags lazyDiagnostics for k, dst := range t.Properties { src, ok := src.Properties[k] if !ok { @@ -221,7 +221,7 @@ func (t *ObjectType) conversionFrom(src Type, unifying bool, seen map[Type]struc return conversionKind, diags case *MapType: conversionKind := UnsafeConversion - var diags hcl.Diagnostics + var diags lazyDiagnostics for _, dst := range t.Properties { if ck, why := dst.conversionFrom(src.ElementType, unifying, seen); ck < conversionKind { conversionKind, diags = ck, why @@ -232,7 +232,7 @@ func (t *ObjectType) conversionFrom(src Type, unifying bool, seen map[Type]struc } return conversionKind, diags } - return NoConversion, hcl.Diagnostics{typeNotConvertible(t, src)} + return NoConversion, func() hcl.Diagnostics { return hcl.Diagnostics{typeNotConvertible(t, src)} } }) } diff --git a/pkg/codegen/hcl2/model/type_opaque.go b/pkg/codegen/hcl2/model/type_opaque.go index 52c26b269..28e2eab28 100644 --- a/pkg/codegen/hcl2/model/type_opaque.go +++ b/pkg/codegen/hcl2/model/type_opaque.go @@ -96,8 +96,8 @@ func (t *OpaqueType) AssignableFrom(src Type) bool { } func (t *OpaqueType) conversionFromImpl( - src Type, unifying, checkUnsafe bool, seen map[Type]struct{}) (ConversionKind, hcl.Diagnostics) { - return conversionFrom(t, src, unifying, seen, func() (ConversionKind, hcl.Diagnostics) { + src Type, unifying, checkUnsafe bool, seen map[Type]struct{}) (ConversionKind, lazyDiagnostics) { + return conversionFrom(t, src, unifying, seen, func() (ConversionKind, lazyDiagnostics) { if constType, ok := src.(*ConstType); ok { return t.conversionFrom(constType.Type, unifying, seen) } @@ -150,7 +150,7 @@ func (t *OpaqueType) conversionFromImpl( }) } -func (t *OpaqueType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, hcl.Diagnostics) { +func (t *OpaqueType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, lazyDiagnostics) { return t.conversionFromImpl(src, unifying, true, seen) } diff --git a/pkg/codegen/hcl2/model/type_output.go b/pkg/codegen/hcl2/model/type_output.go index ca9cd63c8..65abd77f3 100644 --- a/pkg/codegen/hcl2/model/type_output.go +++ b/pkg/codegen/hcl2/model/type_output.go @@ -81,8 +81,8 @@ func (t *OutputType) ConversionFrom(src Type) ConversionKind { return kind } -func (t *OutputType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, hcl.Diagnostics) { - return conversionFrom(t, src, unifying, seen, func() (ConversionKind, hcl.Diagnostics) { +func (t *OutputType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, lazyDiagnostics) { + return conversionFrom(t, src, unifying, seen, func() (ConversionKind, lazyDiagnostics) { switch src := src.(type) { case *OutputType: return t.ElementType.conversionFrom(src.ElementType, unifying, seen) diff --git a/pkg/codegen/hcl2/model/type_promise.go b/pkg/codegen/hcl2/model/type_promise.go index d7cd46f0f..5b558b302 100644 --- a/pkg/codegen/hcl2/model/type_promise.go +++ b/pkg/codegen/hcl2/model/type_promise.go @@ -79,8 +79,8 @@ func (t *PromiseType) ConversionFrom(src Type) ConversionKind { } func (t *PromiseType) conversionFrom( - src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, hcl.Diagnostics) { - return conversionFrom(t, src, unifying, seen, func() (ConversionKind, hcl.Diagnostics) { + src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, lazyDiagnostics) { + return conversionFrom(t, src, unifying, seen, func() (ConversionKind, lazyDiagnostics) { if src, ok := src.(*PromiseType); ok { return t.ElementType.conversionFrom(src.ElementType, unifying, seen) } diff --git a/pkg/codegen/hcl2/model/type_set.go b/pkg/codegen/hcl2/model/type_set.go index 673d14707..8b2180741 100644 --- a/pkg/codegen/hcl2/model/type_set.go +++ b/pkg/codegen/hcl2/model/type_set.go @@ -76,8 +76,8 @@ func (t *SetType) ConversionFrom(src Type) ConversionKind { return kind } -func (t *SetType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, hcl.Diagnostics) { - return conversionFrom(t, src, unifying, seen, func() (ConversionKind, hcl.Diagnostics) { +func (t *SetType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, lazyDiagnostics) { + return conversionFrom(t, src, unifying, seen, func() (ConversionKind, lazyDiagnostics) { switch src := src.(type) { case *SetType: return t.ElementType.conversionFrom(src.ElementType, unifying, seen) @@ -94,7 +94,7 @@ func (t *SetType) conversionFrom(src Type, unifying bool, seen map[Type]struct{} } return UnsafeConversion, nil } - return NoConversion, hcl.Diagnostics{typeNotConvertible(t, src)} + return NoConversion, func() hcl.Diagnostics { return hcl.Diagnostics{typeNotConvertible(t, src)} } }) } diff --git a/pkg/codegen/hcl2/model/type_tuple.go b/pkg/codegen/hcl2/model/type_tuple.go index 03ea7d824..f268ed88f 100644 --- a/pkg/codegen/hcl2/model/type_tuple.go +++ b/pkg/codegen/hcl2/model/type_tuple.go @@ -152,8 +152,8 @@ func (t *TupleType) ConversionFrom(src Type) ConversionKind { return kind } -func (t *TupleType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, hcl.Diagnostics) { - return conversionFrom(t, src, unifying, seen, func() (ConversionKind, hcl.Diagnostics) { +func (t *TupleType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, lazyDiagnostics) { + return conversionFrom(t, src, unifying, seen, func() (ConversionKind, lazyDiagnostics) { switch src := src.(type) { case *TupleType: // When unifying, we will unify two tuples of different length to a new tuple, where elements with matching @@ -166,11 +166,11 @@ func (t *TupleType) conversionFrom(src Type, unifying bool, seen map[Type]struct } if len(t.ElementTypes) != len(src.ElementTypes) { - return NoConversion, hcl.Diagnostics{tuplesHaveDifferentLengths(t, src)} + return NoConversion, func() hcl.Diagnostics { return hcl.Diagnostics{tuplesHaveDifferentLengths(t, src)} } } conversionKind := SafeConversion - var diags hcl.Diagnostics + var diags lazyDiagnostics for i, dst := range t.ElementTypes { if ck, why := dst.conversionFrom(src.ElementTypes[i], unifying, seen); ck < conversionKind { conversionKind, diags = ck, why @@ -191,7 +191,7 @@ func (t *TupleType) conversionFrom(src Type, unifying bool, seen map[Type]struct return conversionKind, diags case *ListType: conversionKind := UnsafeConversion - var diags hcl.Diagnostics + var diags lazyDiagnostics for _, t := range t.ElementTypes { if ck, why := t.conversionFrom(src.ElementType, unifying, seen); ck < conversionKind { conversionKind, diags = ck, why @@ -203,7 +203,7 @@ func (t *TupleType) conversionFrom(src Type, unifying bool, seen map[Type]struct return conversionKind, diags case *SetType: conversionKind := UnsafeConversion - var diags hcl.Diagnostics + var diags lazyDiagnostics for _, t := range t.ElementTypes { if ck, why := t.conversionFrom(src.ElementType, unifying, seen); ck < conversionKind { conversionKind, diags = ck, why @@ -214,7 +214,7 @@ func (t *TupleType) conversionFrom(src Type, unifying bool, seen map[Type]struct } return conversionKind, diags } - return NoConversion, hcl.Diagnostics{typeNotConvertible(t, src)} + return NoConversion, func() hcl.Diagnostics { return hcl.Diagnostics{typeNotConvertible(t, src)} } }) } diff --git a/pkg/codegen/hcl2/model/type_union.go b/pkg/codegen/hcl2/model/type_union.go index 3941e4f86..468106285 100644 --- a/pkg/codegen/hcl2/model/type_union.go +++ b/pkg/codegen/hcl2/model/type_union.go @@ -173,20 +173,36 @@ func (t *UnionType) ConversionFrom(src Type) ConversionKind { return kind } -func (t *UnionType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, hcl.Diagnostics) { - return conversionFrom(t, src, unifying, seen, func() (ConversionKind, hcl.Diagnostics) { +func (t *UnionType) conversionFrom(src Type, unifying bool, seen map[Type]struct{}) (ConversionKind, lazyDiagnostics) { + return conversionFrom(t, src, unifying, seen, func() (ConversionKind, lazyDiagnostics) { var conversionKind ConversionKind - var diags hcl.Diagnostics + var diags []lazyDiagnostics + + // Fast path: see if the source type is equal to any of the element types. Equality checks are generally + // less expensive that full convertibility checks. + for _, t := range t.ElementTypes { + if src.Equals(t) { + return SafeConversion, nil + } + } + for _, t := range t.ElementTypes { ck, why := t.conversionFrom(src, unifying, seen) if ck > conversionKind { conversionKind = ck - } else { - diags = diags.Extend(why) + } else if why != nil { + diags = append(diags, why) } } if conversionKind == NoConversion { - return NoConversion, diags + return NoConversion, func() hcl.Diagnostics { + var all hcl.Diagnostics + for _, why := range diags { + //nolint:errcheck + all.Extend(why()) + } + return all + } } return conversionKind, nil }) @@ -195,7 +211,7 @@ func (t *UnionType) conversionFrom(src Type, unifying bool, seen map[Type]struct // If all conversions to a dest type from a union type are safe, the conversion is safe. // If no conversions to a dest type from a union type exist, the conversion does not exist. // Otherwise, the conversion is unsafe. -func (t *UnionType) conversionTo(dest Type, unifying bool, seen map[Type]struct{}) (ConversionKind, hcl.Diagnostics) { +func (t *UnionType) conversionTo(dest Type, unifying bool, seen map[Type]struct{}) (ConversionKind, lazyDiagnostics) { conversionKind, exists := SafeConversion, false for _, t := range t.ElementTypes { switch kind, _ := dest.conversionFrom(t, unifying, seen); kind {