From e89af80c7b6c1bf6b39f3f78ff965a54c4fdcd34 Mon Sep 17 00:00:00 2001 From: Matthew Jeffryes Date: Tue, 3 Oct 2023 14:18:27 -0700 Subject: [PATCH 1/6] WIP: test potential MustApplyAutoAliases speedup --- pkg/tfbridge/auto_aliasing.go | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/pkg/tfbridge/auto_aliasing.go b/pkg/tfbridge/auto_aliasing.go index f78ea071a..986abc897 100644 --- a/pkg/tfbridge/auto_aliasing.go +++ b/pkg/tfbridge/auto_aliasing.go @@ -175,11 +175,13 @@ func (info *ProviderInfo) ApplyAutoAliases() error { f() } - if err := md.Set(artifact, aliasMetadataKey, hist); err != nil { - // Set fails only when `hist` is not serializable. Because `hist` is - // composed of marshallable, non-cyclic types, this is impossible. - contract.AssertNoErrorf(err, "History failed to serialize") - } + /* + if err := md.Set(artifact, aliasMetadataKey, hist); err != nil { + // Set fails only when `hist` is not serializable. Because `hist` is + // composed of marshallable, non-cyclic types, this is impossible. + contract.AssertNoErrorf(err, "History failed to serialize") + } + */ return nil } @@ -208,11 +210,13 @@ func aliasResource( ) { prev, hasPrev := hist[tfToken] if !hasPrev { - // It's not in the history, so it must be new. Stick it in the history for - // next time. - hist[tfToken] = &tokenHistory[tokens.Type]{ - Current: computed.Tok, - } + /* + // It's not in the history, so it must be new. Stick it in the history for + // next time. + hist[tfToken] = &tokenHistory[tokens.Type]{ + Current: computed.Tok, + } + */ } else { // We don't do this eagerly because aliasResource is called while // iterating over p.Resources which aliasOrRenameResource mutates. @@ -226,7 +230,7 @@ func aliasResource( // Note: If the user explicitly sets a MaxItemOne value, that value is respected // and overwrites the current history.' - if res == nil { + if res == nil || hist[tfToken] == nil { return } @@ -261,9 +265,11 @@ func applyResourceMaxItemsOneAliasing( hasH = hasH || fieldHasHist hasI = hasI || fieldHasInfo - if !hasH { - delete(*hist, k) - } + /* + if !hasH { + delete(*hist, k) + } + */ if !hasI { delete(*info, k) } From 62ced789eee937957ddf8fc4c33706ec2c313b98 Mon Sep 17 00:00:00 2001 From: Matthew Jeffryes Date: Tue, 3 Oct 2023 15:36:26 -0700 Subject: [PATCH 2/6] Disable generating autoaliases if the BuildInfo.path doesn't say we're in tfgen --- pkg/tfbridge/auto_aliasing.go | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/pkg/tfbridge/auto_aliasing.go b/pkg/tfbridge/auto_aliasing.go index 986abc897..1fd81a402 100644 --- a/pkg/tfbridge/auto_aliasing.go +++ b/pkg/tfbridge/auto_aliasing.go @@ -16,6 +16,8 @@ package tfbridge import ( "github.com/Masterminds/semver" + "runtime/debug" + "strings" shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" md "github.com/pulumi/pulumi-terraform-bridge/v3/unstable/metadata" @@ -123,6 +125,9 @@ func (info *ProviderInfo) ApplyAutoAliases() error { return err } + buildInfo, _ := debug.ReadBuildInfo() + isTfgen := buildInfo != nil && strings.Contains(buildInfo.Path, "pulumi-tfgen") + var currentVersion int // If version is missing, we assume the current version is the most recent major // version in mentioned in history. @@ -162,7 +167,7 @@ func (info *ProviderInfo) ApplyAutoAliases() error { for tfToken, computed := range info.Resources { r, _ := rMap.GetOk(tfToken) aliasResource(info, r, &applyAliases, hist.Resources, - computed, tfToken, currentVersion) + computed, tfToken, currentVersion, isTfgen) } for tfToken, computed := range info.DataSources { @@ -175,13 +180,13 @@ func (info *ProviderInfo) ApplyAutoAliases() error { f() } - /* + if isTfgen { if err := md.Set(artifact, aliasMetadataKey, hist); err != nil { // Set fails only when `hist` is not serializable. Because `hist` is // composed of marshallable, non-cyclic types, this is impossible. contract.AssertNoErrorf(err, "History failed to serialize") } - */ + } return nil } @@ -207,16 +212,17 @@ func aliasResource( applyResourceAliases *[]func(), hist map[string]*tokenHistory[tokens.Type], computed *ResourceInfo, tfToken string, version int, + isTfgen bool, ) { prev, hasPrev := hist[tfToken] if !hasPrev { - /* + if isTfgen { // It's not in the history, so it must be new. Stick it in the history for // next time. hist[tfToken] = &tokenHistory[tokens.Type]{ Current: computed.Tok, } - */ + } } else { // We don't do this eagerly because aliasResource is called while // iterating over p.Resources which aliasOrRenameResource mutates. @@ -265,11 +271,9 @@ func applyResourceMaxItemsOneAliasing( hasH = hasH || fieldHasHist hasI = hasI || fieldHasInfo - /* - if !hasH { - delete(*hist, k) - } - */ + if !hasH { + delete(*hist, k) + } if !hasI { delete(*info, k) } From 08c845f5c2cafc956dd2ce8a6bb829e8cba54f98 Mon Sep 17 00:00:00 2001 From: Matthew Jeffryes Date: Wed, 4 Oct 2023 10:21:49 -0700 Subject: [PATCH 3/6] Try using a global --- pkg/tfbridge/auto_aliasing.go | 49 ++++++++++++++++++++--------------- pkg/tfbridge/runtime.go | 48 ++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 21 deletions(-) create mode 100644 pkg/tfbridge/runtime.go diff --git a/pkg/tfbridge/auto_aliasing.go b/pkg/tfbridge/auto_aliasing.go index 1fd81a402..2713492ea 100644 --- a/pkg/tfbridge/auto_aliasing.go +++ b/pkg/tfbridge/auto_aliasing.go @@ -16,8 +16,6 @@ package tfbridge import ( "github.com/Masterminds/semver" - "runtime/debug" - "strings" shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" md "github.com/pulumi/pulumi-terraform-bridge/v3/unstable/metadata" @@ -125,9 +123,6 @@ func (info *ProviderInfo) ApplyAutoAliases() error { return err } - buildInfo, _ := debug.ReadBuildInfo() - isTfgen := buildInfo != nil && strings.Contains(buildInfo.Path, "pulumi-tfgen") - var currentVersion int // If version is missing, we assume the current version is the most recent major // version in mentioned in history. @@ -167,7 +162,7 @@ func (info *ProviderInfo) ApplyAutoAliases() error { for tfToken, computed := range info.Resources { r, _ := rMap.GetOk(tfToken) aliasResource(info, r, &applyAliases, hist.Resources, - computed, tfToken, currentVersion, isTfgen) + computed, tfToken, currentVersion) } for tfToken, computed := range info.DataSources { @@ -180,7 +175,7 @@ func (info *ProviderInfo) ApplyAutoAliases() error { f() } - if isTfgen { + if isTfgen() { if err := md.Set(artifact, aliasMetadataKey, hist); err != nil { // Set fails only when `hist` is not serializable. Because `hist` is // composed of marshallable, non-cyclic types, this is impossible. @@ -212,11 +207,10 @@ func aliasResource( applyResourceAliases *[]func(), hist map[string]*tokenHistory[tokens.Type], computed *ResourceInfo, tfToken string, version int, - isTfgen bool, ) { prev, hasPrev := hist[tfToken] if !hasPrev { - if isTfgen { + if isTfgen() { // It's not in the history, so it must be new. Stick it in the history for // next time. hist[tfToken] = &tokenHistory[tokens.Type]{ @@ -271,7 +265,7 @@ func applyResourceMaxItemsOneAliasing( hasH = hasH || fieldHasHist hasI = hasI || fieldHasInfo - if !hasH { + if !hasH && isTfgen() { delete(*hist, k) } if !hasI { @@ -330,13 +324,15 @@ func applyMaxItemsOneAliasing(schema shim.Schema, h *fieldHistory, info *SchemaI // MaxItemsOne does not apply, so do nothing } else if info.MaxItemsOne != nil { // The user has overwritten the value, so we will just record that. - h.MaxItemsOne = info.MaxItemsOne - hasH = true + if isTfgen() { + h.MaxItemsOne = info.MaxItemsOne + hasH = true + } } else if h.MaxItemsOne != nil { // If we have a previous value in the history, we keep it as is. info.MaxItemsOne = h.MaxItemsOne hasI = true - } else { + } else if isTfgen() { // There is no history for this value, so we bake it into the // alias history. h.MaxItemsOne = BoolRef(IsMaxItemsOne(schema, info)) @@ -349,10 +345,12 @@ func applyMaxItemsOneAliasing(schema shim.Schema, h *fieldHistory, info *SchemaI // If the .Elem existed before this function, we mark it as unsafe to cleanup. var hasElemH, hasElemI bool populateElem := func() { - if h.Elem == nil { - h.Elem = &fieldHistory{} - } else { - hasElemH = true + if isTfgen() { + if h.Elem == nil { + h.Elem = &fieldHistory{} + } else { + hasElemH = true + } } if info.Elem == nil { info.Elem = &SchemaInfo{} @@ -367,7 +365,7 @@ func applyMaxItemsOneAliasing(schema shim.Schema, h *fieldHistory, info *SchemaI cleanupElem := func(elemHist, elemInfo bool) { hasElemH = hasElemH || elemHist hasElemI = hasElemI || elemInfo - if !hasElemH { + if !hasElemH && isTfgen() { h.Elem = nil } if !hasElemI { @@ -379,11 +377,17 @@ func applyMaxItemsOneAliasing(schema shim.Schema, h *fieldHistory, info *SchemaI switch e := e.(type) { case shim.Resource: populateElem() - eHasH, eHasI := applyResourceMaxItemsOneAliasing(e, &h.Elem.Fields, &info.Elem.Fields) + var eHasH, eHasI bool + if h.Elem != nil { + eHasH, eHasI = applyResourceMaxItemsOneAliasing(e, &h.Elem.Fields, &info.Elem.Fields) + } cleanupElem(eHasH, eHasI) case shim.Schema: populateElem() - eHasH, eHasI := applyMaxItemsOneAliasing(e, h.Elem, info.Elem) + var eHasH, eHasI bool + if h.Elem != nil { + eHasH, eHasI = applyMaxItemsOneAliasing(e, h.Elem, info.Elem) + } cleanupElem(eHasH, eHasI) } @@ -477,7 +481,6 @@ func aliasDataSource( } applyResourceMaxItemsOneAliasing(ds, &hist[tfToken].Fields, &computed.Fields) - } func aliasOrRenameDataSource( @@ -518,3 +521,7 @@ func aliasOrRenameDataSource( } } + +func isTfgen() bool { + return GetRuntimeStage() != ResourceStage +} diff --git a/pkg/tfbridge/runtime.go b/pkg/tfbridge/runtime.go new file mode 100644 index 000000000..b2de6e82c --- /dev/null +++ b/pkg/tfbridge/runtime.go @@ -0,0 +1,48 @@ +package tfbridge + +import ( + "runtime/debug" + "strings" +) + +type RuntimeStage int + +const ( + UnknownStage RuntimeStage = iota + TfgenStage + ResourceStage +) + +// Holds runtime flags +type RuntimeInfo struct { + Stage RuntimeStage +} + +var runtime = initRuntimeInfo() + +func initRuntimeInfo() RuntimeInfo { + buildInfo, _ := debug.ReadBuildInfo() + stage := UnknownStage + if buildInfo != nil { + if strings.Contains(buildInfo.Path, "pulumi-tfgen") { + stage = TfgenStage + } else if strings.Contains(buildInfo.Path, "pulumi-resource") { + stage = ResourceStage + } + } + return RuntimeInfo{ + Stage: stage, + } +} + +func ReadRuntimeInfo() RuntimeInfo { + return runtime +} + +func GetRuntimeStage() RuntimeStage { + return runtime.Stage +} + +func SetRuntimeStage(s RuntimeStage) { + runtime.Stage = s +} From 2b391da22340107b27891ceb795537d10413cca7 Mon Sep 17 00:00:00 2001 From: Matthew Jeffryes Date: Fri, 6 Oct 2023 14:09:17 -0700 Subject: [PATCH 4/6] Make the runtime info package local --- pkg/tfbridge/auto_aliasing.go | 2 +- pkg/tfbridge/runtime.go | 34 +++++++++++++++++----------------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/pkg/tfbridge/auto_aliasing.go b/pkg/tfbridge/auto_aliasing.go index 2713492ea..2ed2d56f5 100644 --- a/pkg/tfbridge/auto_aliasing.go +++ b/pkg/tfbridge/auto_aliasing.go @@ -523,5 +523,5 @@ func aliasOrRenameDataSource( } func isTfgen() bool { - return GetRuntimeStage() != ResourceStage + return getRuntimeStage() != resourceStage } diff --git a/pkg/tfbridge/runtime.go b/pkg/tfbridge/runtime.go index b2de6e82c..d64136a0a 100644 --- a/pkg/tfbridge/runtime.go +++ b/pkg/tfbridge/runtime.go @@ -5,44 +5,44 @@ import ( "strings" ) -type RuntimeStage int +type runtimeStage int const ( - UnknownStage RuntimeStage = iota - TfgenStage - ResourceStage + unknownStage runtimeStage = iota + tfgenStage + resourceStage ) // Holds runtime flags -type RuntimeInfo struct { - Stage RuntimeStage +type runtimeInfo struct { + stage runtimeStage } var runtime = initRuntimeInfo() -func initRuntimeInfo() RuntimeInfo { +func initRuntimeInfo() runtimeInfo { buildInfo, _ := debug.ReadBuildInfo() - stage := UnknownStage + stage := unknownStage if buildInfo != nil { if strings.Contains(buildInfo.Path, "pulumi-tfgen") { - stage = TfgenStage + stage = tfgenStage } else if strings.Contains(buildInfo.Path, "pulumi-resource") { - stage = ResourceStage + stage = resourceStage } } - return RuntimeInfo{ - Stage: stage, + return runtimeInfo{ + stage: stage, } } -func ReadRuntimeInfo() RuntimeInfo { +func readRuntimeInfo() runtimeInfo { return runtime } -func GetRuntimeStage() RuntimeStage { - return runtime.Stage +func getRuntimeStage() runtimeStage { + return runtime.stage } -func SetRuntimeStage(s RuntimeStage) { - runtime.Stage = s +func setRuntimeStage(s runtimeStage) { + runtime.stage = s } From 5424cad6ebf93d3cf7b557bc1d75b817268fec3e Mon Sep 17 00:00:00 2001 From: Matthew Jeffryes Date: Mon, 9 Oct 2023 14:21:12 -0700 Subject: [PATCH 5/6] appease linter --- pkg/tfbridge/runtime.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/pkg/tfbridge/runtime.go b/pkg/tfbridge/runtime.go index d64136a0a..269238b4f 100644 --- a/pkg/tfbridge/runtime.go +++ b/pkg/tfbridge/runtime.go @@ -35,14 +35,6 @@ func initRuntimeInfo() runtimeInfo { } } -func readRuntimeInfo() runtimeInfo { - return runtime -} - func getRuntimeStage() runtimeStage { return runtime.stage } - -func setRuntimeStage(s runtimeStage) { - runtime.stage = s -} From d74f91f8cd3946ac260fb1767202d176ff3ea412 Mon Sep 17 00:00:00 2001 From: Matthew Jeffryes Date: Tue, 10 Oct 2023 14:23:14 -0700 Subject: [PATCH 6/6] simplify --- pkg/tfbridge/auto_aliasing.go | 31 ++++++++++++------------------- pkg/tfbridge/runtime.go | 4 ++-- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/pkg/tfbridge/auto_aliasing.go b/pkg/tfbridge/auto_aliasing.go index 2ed2d56f5..f79adf2d8 100644 --- a/pkg/tfbridge/auto_aliasing.go +++ b/pkg/tfbridge/auto_aliasing.go @@ -339,18 +339,21 @@ func applyMaxItemsOneAliasing(schema shim.Schema, h *fieldHistory, info *SchemaI hasH = true } + // if h.Elem is null and we're not trying to generate updated history, we're done + if h.Elem == nil && !isTfgen() { + return hasH, hasI + } + // Ensure that the h.Elem and info.Elem fields are non-nil so they can be // safely recursed on. // // If the .Elem existed before this function, we mark it as unsafe to cleanup. var hasElemH, hasElemI bool populateElem := func() { - if isTfgen() { - if h.Elem == nil { - h.Elem = &fieldHistory{} - } else { - hasElemH = true - } + if h.Elem == nil { + h.Elem = &fieldHistory{} + } else { + hasElemH = true } if info.Elem == nil { info.Elem = &SchemaInfo{} @@ -365,7 +368,7 @@ func applyMaxItemsOneAliasing(schema shim.Schema, h *fieldHistory, info *SchemaI cleanupElem := func(elemHist, elemInfo bool) { hasElemH = hasElemH || elemHist hasElemI = hasElemI || elemInfo - if !hasElemH && isTfgen() { + if !hasElemH { h.Elem = nil } if !hasElemI { @@ -377,17 +380,11 @@ func applyMaxItemsOneAliasing(schema shim.Schema, h *fieldHistory, info *SchemaI switch e := e.(type) { case shim.Resource: populateElem() - var eHasH, eHasI bool - if h.Elem != nil { - eHasH, eHasI = applyResourceMaxItemsOneAliasing(e, &h.Elem.Fields, &info.Elem.Fields) - } + eHasH, eHasI := applyResourceMaxItemsOneAliasing(e, &h.Elem.Fields, &info.Elem.Fields) cleanupElem(eHasH, eHasI) case shim.Schema: populateElem() - var eHasH, eHasI bool - if h.Elem != nil { - eHasH, eHasI = applyMaxItemsOneAliasing(e, h.Elem, info.Elem) - } + eHasH, eHasI := applyMaxItemsOneAliasing(e, h.Elem, info.Elem) cleanupElem(eHasH, eHasI) } @@ -521,7 +518,3 @@ func aliasOrRenameDataSource( } } - -func isTfgen() bool { - return getRuntimeStage() != resourceStage -} diff --git a/pkg/tfbridge/runtime.go b/pkg/tfbridge/runtime.go index 269238b4f..b708d659c 100644 --- a/pkg/tfbridge/runtime.go +++ b/pkg/tfbridge/runtime.go @@ -35,6 +35,6 @@ func initRuntimeInfo() runtimeInfo { } } -func getRuntimeStage() runtimeStage { - return runtime.stage +func isTfgen() bool { + return runtime.stage != resourceStage }