types/basetypes: Reduce set duplicate detection and equality to linear time - #1323
Open
samssh wants to merge 1 commit into
Open
types/basetypes: Reduce set duplicate detection and equality to linear time#1323samssh wants to merge 1 commit into
samssh wants to merge 1 commit into
Conversation
|
Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement Learn more about why HashiCorp requires a CLA and what the CLA includes AmirMohammad Sadat Shokouhi seems not to be a GitHub user. Have you signed the CLA already but the status is still pending? Recheck it. |
…tion and equality SetType.Validate compared every pair of elements to detect duplicates, and SetValue.Equal scanned the whole other set for each element, both O(n^2) deep tftypes.Value comparisons. SetValue.contains additionally called Elements(), which defensively copies the entire element slice on every call, so comparing two n element sets allocated n slices of length n. Group elements by their String() output, which is equal whenever Equal is, and run the deep comparison only within a group. Element types with a set (or a dynamic type) nested anywhere inside them keep the pairwise comparison, because set equality ignores element order while String() does not. The existing SetType.Validate benchmarks ranged over an empty slice and so measured an empty set; populate it and add matching SetValue.Equal benchmarks.
samssh
force-pushed
the
set-duplicate-and-equality-performance
branch
from
September 3, 2026 07:13
1023809 to
d5ac0a2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
Fixes #1322 (partially — this PR covers sites 1 and 2)
Description
SetType.Validatecompared every pair of elements to find duplicates, andSetValue.Equalscanned the whole other set for each element. Both are O(n²)deep
tftypes.Valuecomparisons, and each of those walks the value twice andallocates an
AttributePathper visited path.SetValue.containsmade itworse by iterating
Elements(), which defensively copies the entire elementslice on every call — so comparing two n-element sets allocated n slices of
length n.
Both now group elements by their
String()output and run the deep comparisononly within a group.
Equalimplies an identicalString(), so nothing can bemissed.
That is not true for every element type: set equality ignores element order
while
SetValue.String()renders elements in slice order, so two equal nestedsets can render differently.
typeCanBeStringKeyedwalks the element type treeonce and returns false if a set — or a dynamic type, whose concrete type is only
known at runtime — appears anywhere inside it; those keep the original pairwise
comparison. Object types are fine, because
ObjectValue.String()sorts itsattribute names for exactly this reason.
Without that guard the change passes almost the whole suite and fails exactly
one test,
TestValueSemanticEqualitySet/SetValue-SetValue-StringValuableWithSemanticEquals-true-diff-order.With it, the full suite passes unchanged.
The existing
BenchmarkSetTypeValidate*benchmarks did not measure any of this:the helper ranged over an empty slice, so every one of them validated an empty
set. This populates it and adds matching
BenchmarkSetValueEqual*, at-benchtime 1x:SetTypeValidate1000SetTypeValidate10000SetValueEqual1000SetValueEqual10000SetType.Validatenow compares each element against the earlier elementssharing its key rather than the later ones, which reports the same duplicates
the same number of times.
Rollback Plan
Changes to Security Controls
No.