Skip to content

Index indexed children rather than scanning per element - #51556

Open
tommyk-gears wants to merge 2 commits into
spring-projects:mainfrom
tommyk-gears:gh-children-of-folded
Open

Index indexed children rather than scanning per element#51556
tommyk-gears wants to merge 2 commits into
spring-projects:mainfrom
tommyk-gears:gh-children-of-folded

Conversation

@tommyk-gears

Copy link
Copy Markdown
Contributor

IndexedElementsBinder discovers which indices exist beneath a name by scanning every
property name in the source:

for (ConfigurationPropertyName name : source.filter(root::isAncestorOf)) {
    ConfigurationPropertyName choppedName = name.chop(root.getNumberOfElements() + 1);
    if (choppedName.isLastElementIndexed()) { ... }
}

The indexed property-name syntax carries no length, so the only evidence that tags[0]
exists is that some name happens to look like tags[0].name. That means one full scan per
indexed element. When many indexed elements share a namespace — a large Map whose values
contain lists, say — binding becomes quadratic in the number of elements.

Binding a Map whose values each contain two lists:

entries before after
100 22.7ms 12.7ms
400 130.9ms 21.3ms
800 456.5ms 30.1ms
1600 1879ms 59ms

The per-element cost stops growing with the size of the source.

Changes

Add getChildrenOf to IterableConfigurationPropertySource. The default implementation
performs the same scan as before, so sources that cannot answer more cheaply are unaffected.
SpringIterableConfigurationPropertySource overrides it and answers from mappings built
during the pass its cache already makes over property names.

Answer containsDescendantOf from those mappings. The cache built a separate descendants
set purely so containsDescendantOf could avoid scanning. The children mappings have the
same key set, so that set is now redundant and is removed along with addParents and its
capture flag.

Like addParents before it, addChildren stops walking as soon as it reaches a parent that
is already known — whichever name first recorded that parent walked the rest of the way up.
Without that short-circuit the mappings take roughly twice as long to build. With it,
building the mappings for 6000 property names takes 3.6-4.2ms, against 3.6-4.1ms for the
descendants set they replace.

The ancestorOfCheck guard is kept in containsDescendantOf, since a mapper with a custom
check (SystemEnvironmentPropertyMapper) still needs its own ancestor logic. The children
mappings themselves are built for every source, because getChildrenOf needs them
regardless of which mapper is in use.

Notes

  • getChildrenOf has one caller today. I looked at the other isAncestorOf/isParentOf
    sites (NoUnboundElementsBindHandler, ValidationBindHandler,
    assertNoUnboundChildren); they all want descendants rather than direct children, so
    none migrate cleanly.
  • Memory use of the children mappings is not measured. They hold a Set per parent where
    the descendants set held one flat set of parents.
  • Tests cover direct children, agreement with the default implementation, equivalent names
    written differently (foo-bar / fooBar), and system environment sources.

Binding an indexed element scanned every property name in the source to
discover which indices exist, using filter(root::isAncestorOf). With many
indexed elements in a single namespace, for example a large Map whose values
contain lists, this made binding quadratic in the number of elements.

Add a getChildrenOf method that returns the names directly beneath a given
name. The default implementation performs the same scan as before, so
sources that cannot answer more cheaply are unaffected.
SpringIterableConfigurationPropertySource overrides it and answers from
mappings built during the pass its cache already makes over property names,
turning each lookup into a map access.

The children mappings are kept separate from the existing descendants set
for now. They have the same key set, so containsDescendantOf could later be
answered from them, but that is left alone here to keep this change small.

Binding a Map of 717 entries whose values each contain two lists drops from
133ms to 30ms (best of 25 runs, measured either side of this commit).

Signed-off-by: Tommy Karlsson <tommy.karlsson@leovegas.com>
The cache built a descendants set purely so that containsDescendantOf could
answer without scanning. Now that it also records each name against its
parent, that set is redundant: the children mappings have the same key set,
so containsDescendantOf can ask whether a name has any children.

Removes the descendants set, addParents and the capture flag. The children
mappings are always built, since getChildrenOf needs them for every source,
and the ancestorOfCheck guard is kept in containsDescendantOf, where a
mapper with a custom check still needs its own ancestor logic.

Like addParents before it, addChildren stops walking as soon as it reaches a
parent that is already known. Whichever name first recorded that parent
walked the rest of the way up, so everything above it is already present.
Without that the mappings would take roughly twice as long to build.

Building the mappings for 6000 property names takes 3.6-4.2ms, against
3.6-4.1ms for the descendants set it replaces.

Signed-off-by: Tommy Karlsson <tommy.karlsson@leovegas.com>
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: waiting-for-triage An issue we've not yet triaged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants