From f243754a12e9bc270dd8d2a0e74b5a8b2bc1e6ab Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 22 May 2025 09:11:22 -0400 Subject: [PATCH 1/2] reduce changes lookups in GetResource Expanded resources always require fetching all instances for evaluation, but the changes data structure requires iterating over all instances for each call to GetResourceInstanceChange, so it turns out to be an n^2 operation. This wasn't very visible in most configs with relatively few numbers of instances, but when you have a large number of instances referencing another resource with a large number of instances (e.g. the second resource uses the first as the for_each argument), you end up with (n^2)^2 lookups. Because the instance changes are in a global structure, this single point of lock contention would hold up the entire plan. --- internal/terraform/evaluate.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go index 095830db79..e3151ea654 100644 --- a/internal/terraform/evaluate.go +++ b/internal/terraform/evaluate.go @@ -658,6 +658,16 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc instances[key] = value } + // Proactively read out all the resource changes before iteration. Not only + // does GetResourceInstanceChange have to iterate over all instances + // internally causing an n^2 lookup, but Changes is also a major point of + // lock contention. + resChanges := d.Evaluator.Changes.GetChangesForConfigResource(addr.InModule(moduleConfig.Path)) + instChanges := addrs.MakeMap[addrs.AbsResourceInstance, *plans.ResourceInstanceChange]() + for _, ch := range resChanges { + instChanges.Put(ch.Addr, ch) + } + // Decode all instances in the current state pendingDestroy := d.Operation == walkDestroy for key, is := range rs.Instances { @@ -675,7 +685,7 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc } instAddr := addr.Instance(key).Absolute(d.ModulePath) - change := d.Evaluator.Changes.GetResourceInstanceChange(instAddr, addrs.NotDeposed) + change := instChanges.Get(instAddr) if change != nil { // Don't take any resources that are yet to be deleted into account. // If the referenced resource is CreateBeforeDestroy, then orphaned From fd6b10c84d42779493d640ac903986e903eb262c Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 22 May 2025 09:31:22 -0400 Subject: [PATCH 2/2] CHANGELOG --- .changes/v1.13/ENHANCEMENTS-20250522-093102.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changes/v1.13/ENHANCEMENTS-20250522-093102.yaml diff --git a/.changes/v1.13/ENHANCEMENTS-20250522-093102.yaml b/.changes/v1.13/ENHANCEMENTS-20250522-093102.yaml new file mode 100644 index 0000000000..5363103a43 --- /dev/null +++ b/.changes/v1.13/ENHANCEMENTS-20250522-093102.yaml @@ -0,0 +1,5 @@ +kind: ENHANCEMENTS +body: Performance fix for evaluating high cardinality resources +time: 2025-05-22T09:31:02.761383-04:00 +custom: + Issue: "26355"