Skip to content

Commit

Permalink
Optimize startup performance (#3070)
Browse files Browse the repository at this point in the history 
Zooming in a lot on AWS Provider startup costs, noticing a recurring
pattern. In SDKv2 we have schema.Resource that has Schema of type map
but also SchemaFunc The latter is designed specifically to defer / lazy
instantiate schemas that are big. It looks like in AWS QuickSight and
some other very large schemas make use of this. However over time
transformations baked into upstream providers and our own bridge added
up to a lot of passes that force-call these SchemaFunc() on startup,
even if you don't use QuickSight. When done sequentially it all adds up.
Moreover since there's no true laziness we end up paying the QuickSight
etc cost multiple times.

This change does the following to avoid `SchemaFunc()` calls:

- turn off upstream tags check - this seems to be a validation-only
measure, that somehow manages to call SchemaMap and SchemaFunc and slow
things down; it now disabled in the bridged provider

- rewrite markTagsAllNotComputedForResources to avoid calling
`SchemaMap()` eagerly; instead call on-demand and try to cache the call
so QuickSight and friends are not initialized twice
  • Loading branch information
@t0yv0
t0yv0 authored Dec 2, 2023
1 parent f37b7cc commit db55339
Showing 1 changed file with 163 additions and 0 deletions.
163 changes: 163 additions & 0 deletions patches/0031-Optimize-startup-performance.patch
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Anton Tayanovskyy <[email protected]>
Date: Thu, 30 Nov 2023 14:28:37 -0500
Subject: [PATCH 31/31] Optimize startup performance


diff --git a/internal/provider/provider.go b/internal/provider/provider.go
index 60554782a2..1017fd2bb6 100644
--- a/internal/provider/provider.go
+++ b/internal/provider/provider.go
@@ -306,7 +306,7 @@ func New(ctx context.Context) (*schema.Provider, error) {
interceptors := interceptorItems{}

if v.Tags != nil {
- schema := r.SchemaMap()
+ schema := schemaMapForTagsChecking(ctx, r, true)

// The data source has opted in to transparent tagging.
// Ensure that the schema look OK.
@@ -383,7 +383,7 @@ func New(ctx context.Context) (*schema.Provider, error) {
interceptors := interceptorItems{}

if v.Tags != nil {
- schema := r.SchemaMap()
+ schema := schemaMapForTagsChecking(ctx, r, false)

// The resource has opted in to transparent tagging.
// Ensure that the schema look OK.
diff --git a/internal/provider/provider_tagcheck.go b/internal/provider/provider_tagcheck.go
new file mode 100644
index 0000000000..35202ebd58
--- /dev/null
+++ b/internal/provider/provider_tagcheck.go
@@ -0,0 +1,28 @@
+package provider
+
+import (
+ "context"
+
+ "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+ "github.com/hashicorp/terraform-provider-aws/names"
+)
+
+type disableTagsSchemaCheckKey struct{}
+
+func DisableTagSchemaCheck(ctx context.Context) context.Context {
+ return context.WithValue(ctx, disableTagsSchemaCheckKey{}, true)
+}
+
+func schemaMapForTagsChecking(ctx context.Context, r *schema.Resource, tagsComputed bool) map[string]*schema.Schema {
+ flag := ctx.Value(disableTagsSchemaCheckKey{})
+ switch flag := flag.(type) {
+ case bool:
+ if flag {
+ return map[string]*schema.Schema{
+ names.AttrTags: &schema.Schema{Computed: tagsComputed},
+ names.AttrTagsAll: &schema.Schema{Computed: true},
+ }
+ }
+ }
+ return r.SchemaMap()
+}
diff --git a/shim/shim.go b/shim/shim.go
index e24e53fe17..00297dbe77 100644
--- a/shim/shim.go
+++ b/shim/shim.go
@@ -3,6 +3,7 @@ package shim
import (
"context"
"fmt"
+ "sync"

pfprovider "github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -17,7 +18,7 @@ type UpstreamProvider struct {
}

func NewUpstreamProvider(ctx context.Context) (UpstreamProvider, error) {
- primary, err := provider.New(ctx)
+ primary, err := provider.New(provider.DisableTagSchemaCheck(ctx))
if err != nil {
return UpstreamProvider{}, err
}
@@ -44,44 +45,47 @@ func NewTagConfig(ctx context.Context, i interface{}) TagConfig {
// rationale for this is that Pulumi copies tags to tags_all before it hits the TF layer, so these
// attributes must match in schema.
func markTagsAllNotComputedForResources(sdkV2Provider *schema.Provider) {
-
- updateSchema := func(rn string, s map[string]*schema.Schema) map[string]*schema.Schema {
- updatedSchema := map[string]*schema.Schema{}
- for k, v := range s {
- if k == "tags_all" {
- if tagsSchema, ok := s["tags"]; ok {
- tagsAll := *tagsSchema
- updatedSchema[k] = &tagsAll
- } else {
- panic(fmt.Sprintf("Unable to edit tagsAll schema for %q", rn))
- }
- } else {
- updatedSchema[k] = v
- }
- }
- return updatedSchema
+ updatedResourcesMap := map[string]*schema.Resource{}
+ for rn, r := range sdkV2Provider.ResourcesMap {
+ updatedResourcesMap[rn] = markTagsAllNotComputedForResource(rn, r)
}
+ sdkV2Provider.ResourcesMap = updatedResourcesMap
+}

- updatedResource := func(rn string, r *schema.Resource) *schema.Resource {
- if _, ok := r.SchemaMap()["tags_all"]; !ok {
- return r
+func markTagsAllNotComputedForResource(rn string, r *schema.Resource) *schema.Resource {
+ u := *r
+ if r.SchemaFunc != nil {
+ old := r.SchemaFunc
+ var once sync.Once
+ var cache map[string]*schema.Schema
+ u.SchemaFunc = func() map[string]*schema.Schema {
+ once.Do(func() {
+ cache = markTagsAllNotComputedForSchema(rn, old())
+ })
+ return cache
}
+ } else {
+ u.Schema = markTagsAllNotComputedForSchema(rn, r.Schema)
+ }
+ return &u
+}

- u := *r
- if r.SchemaFunc != nil {
- old := r.SchemaFunc
- u.SchemaFunc = func() map[string]*schema.Schema {
- return updateSchema(rn, old())
+func markTagsAllNotComputedForSchema(rn string, s map[string]*schema.Schema) map[string]*schema.Schema {
+ if _, ok := s["tags_all"]; !ok {
+ return s
+ }
+ updatedSchema := map[string]*schema.Schema{}
+ for k, v := range s {
+ if k == "tags_all" {
+ if tagsSchema, ok := s["tags"]; ok {
+ tagsAll := *tagsSchema
+ updatedSchema[k] = &tagsAll
+ } else {
+ panic(fmt.Sprintf("Unable to edit tagsAll schema for %q", rn))
}
} else {
- u.Schema = updateSchema(rn, r.Schema)
+ updatedSchema[k] = v
}
- return &u
- }
-
- updatedResourcesMap := map[string]*schema.Resource{}
- for rn, r := range sdkV2Provider.ResourcesMap {
- updatedResourcesMap[rn] = updatedResource(rn, r)
}
- sdkV2Provider.ResourcesMap = updatedResourcesMap
+ return updatedSchema
}

0 comments on commit db55339

Please sign in to comment.