Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve performance of AttachResourceConfigTransformer on big graphs #35088

Merged
merged 1 commit into from
Apr 29, 2024
Merged

Improve performance of AttachResourceConfigTransformer on big graphs #35088

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
65 changes: 14 additions & 51 deletions internal/terraform/transform_attach_config_resource.go
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@ package terraform
import (
"log"

"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/configs"
"github.com/hashicorp/terraform/internal/dag"
)
Expand Down Expand Up @@ -48,63 +49,25 @@ func (t *AttachResourceConfigTransformer) Transform(g *Graph) error {
log.Printf("[TRACE] AttachResourceConfigTransformer: %q (%T) has no configuration available", dag.VertexName(v), v)
continue
}

for _, r := range config.Module.ManagedResources {
rAddr := r.Addr()

if rAddr != addr.Resource {
// Not the same resource
continue
}

log.Printf("[TRACE] AttachResourceConfigTransformer: attaching to %q (%T) config from %s", dag.VertexName(v), v, r.DeclRange)
arn.AttachResourceConfig(r)

// attach the provider_meta info
if gnapmc, ok := v.(GraphNodeAttachProviderMetaConfigs); ok {
log.Printf("[TRACE] AttachResourceConfigTransformer: attaching provider meta configs to %s", dag.VertexName(v))
if config == nil {
log.Printf("[TRACE] AttachResourceConfigTransformer: no config set on the transformer for %s", dag.VertexName(v))
continue
}
if config.Module == nil {
log.Printf("[TRACE] AttachResourceConfigTransformer: no module in config for %s", dag.VertexName(v))
continue
}
if config.Module.ProviderMetas == nil {
log.Printf("[TRACE] AttachResourceConfigTransformer: no provider metas defined for %s", dag.VertexName(v))
continue
}
gnapmc.AttachProviderMetaConfigs(config.Module.ProviderMetas)
}
var m map[string]*configs.Resource
if addr.Resource.Mode == addrs.ManagedResourceMode {
m = config.Module.ManagedResources
} else if addr.Resource.Mode == addrs.DataResourceMode {
m = config.Module.DataResources
} else {
panic("unknown resource mode: " + addr.Resource.Mode.String())
}
for _, r := range config.Module.DataResources {
rAddr := r.Addr()

if rAddr != addr.Resource {
// Not the same resource
continue
}

coord := addr.Resource.String()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's "coord" mean in this context? 😅

I guess it doesn't matter, since we just use it as a map key on the next line and never observe it again, but the name was a bit disorienting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it as about coordinate, but key could be a good name as well

if r, ok := m[coord]; ok && r.Addr() == addr.Resource {
log.Printf("[TRACE] AttachResourceConfigTransformer: attaching to %q (%T) config from %#v", dag.VertexName(v), v, r.DeclRange)
arn.AttachResourceConfig(r)

// attach the provider_meta info
if gnapmc, ok := v.(GraphNodeAttachProviderMetaConfigs); ok {
log.Printf("[TRACE] AttachResourceConfigTransformer: attaching provider meta configs to %s", dag.VertexName(v))
if config == nil {
log.Printf("[TRACE] AttachResourceConfigTransformer: no config set on the transformer for %s", dag.VertexName(v))
continue
}
if config.Module == nil {
log.Printf("[TRACE] AttachResourceConfigTransformer: no module in config for %s", dag.VertexName(v))
continue
}
if config.Module.ProviderMetas == nil {
log.Printf("[TRACE] AttachResourceConfigTransformer: no provider metas defined for %s", dag.VertexName(v))
continue
if config.Module.ProviderMetas != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit deletes two additional nil checks (config and config.Module) — for posterity's sake, I'll note that they were both, indeed, useless. 👍🏼 (There's already an early exit on nil config above, and the existing code was already assuming that config.Module is never nil when it iterated over two of Module's fields.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yex. The config deletion was highlighted by VSCode itself

gnapmc.AttachProviderMetaConfigs(config.Module.ProviderMetas)
} else {
log.Printf("[TRACE] AttachResourceConfigTransformer: no provider meta configs available to attach to %s", dag.VertexName(v))
}
gnapmc.AttachProviderMetaConfigs(config.Module.ProviderMetas)
}
}
}
Expand Down