Improve ConfigurationPropertySource performance

Attempt to improve the performance of the `ConfigurationPropertySource`
adapters `containsDescendantOf` method. The method now operates on
arrays rather than iterators and reduces the inner for-loop when
possible.

See gh-21416

Author: philwebb

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/DefaultPropertyMapper.java

@@ -79,11 +79,6 @@ private ConfigurationPropertyName tryMap(String propertySourceName) {
 		return ConfigurationPropertyName.EMPTY;
 	}
 
-	@Override
-	public boolean isAncestorOf(ConfigurationPropertyName name, ConfigurationPropertyName candidate) {
-		return name.isAncestorOf(candidate);
-	}
-
 	private static class LastMapping<T, M> {
 
 		private final T from;

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/PropertyMapper.java

@@ -17,6 +17,7 @@
 package org.springframework.boot.context.properties.source;
 
 import java.util.List;
+import java.util.function.BiPredicate;
 
 import org.springframework.core.env.EnumerablePropertySource;
 import org.springframework.core.env.PropertySource;
@@ -39,6 +40,11 @@
  */
 interface PropertyMapper {
 
+	/**
+	 * The default ancestor of check.
+	 */
+	BiPredicate<ConfigurationPropertyName, ConfigurationPropertyName> DEFAULT_ANCESTOR_OF_CHECK = ConfigurationPropertyName::isAncestorOf;
+
 	/**
 	 * Provide mappings from a {@link ConfigurationPropertySource}
 	 * {@link ConfigurationPropertyName}.
@@ -56,12 +62,12 @@ interface PropertyMapper {
 	ConfigurationPropertyName map(String propertySourceName);
 
 	/**
-	 * Returns {@code true} if {@code name} is an ancestor (immediate or nested parent) of
-	 * the given candidate when considering mapping rules.
-	 * @param name the source name
-	 * @param candidate the candidate to check
-	 * @return {@code true} if the candidate is an ancestor of the name
+	 * Returns a {@link BiPredicate} that can be used to check if one name is an ancestor
+	 * of another when considering the mapping rules.
+	 * @return a predicate that can be used to check if one name is an ancestor of another
 	 */
-	boolean isAncestorOf(ConfigurationPropertyName name, ConfigurationPropertyName candidate);
+	default BiPredicate<ConfigurationPropertyName, ConfigurationPropertyName> getAncestorOfCheck() {
+		return DEFAULT_ANCESTOR_OF_CHECK;
+	}
 
 }

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringConfigurationPropertySource.java

@@ -69,6 +69,7 @@ class SpringConfigurationPropertySource implements ConfigurationPropertySource {
 	 */
 	SpringConfigurationPropertySource(PropertySource<?> propertySource, PropertyMapper... mappers) {
 		Assert.notNull(propertySource, "PropertySource must not be null");
+		Assert.isTrue(mappers.length > 0, "Mappers must contain at least one item");
 		this.propertySource = propertySource;
 		this.mappers = mappers;
 	}

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySource.java

@@ -16,15 +16,16 @@
 
 package org.springframework.boot.context.properties.source;
 
-import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.ConcurrentModificationException;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.function.BiPredicate;
 import java.util.function.Supplier;
 import java.util.stream.Stream;
 
@@ -53,14 +54,27 @@
 class SpringIterableConfigurationPropertySource extends SpringConfigurationPropertySource
 		implements IterableConfigurationPropertySource, CachingConfigurationPropertySource {
 
-	private volatile Collection<ConfigurationPropertyName> configurationPropertyNames;
+	private volatile ConfigurationPropertyName[] configurationPropertyNames;
 
 	private final SoftReferenceConfigurationPropertyCache<Mappings> cache;
 
+	private final BiPredicate<ConfigurationPropertyName, ConfigurationPropertyName> ancestorOfCheck;
+
 	SpringIterableConfigurationPropertySource(EnumerablePropertySource<?> propertySource, PropertyMapper... mappers) {
 		super(propertySource, mappers);
 		assertEnumerablePropertySource();
 		this.cache = new SoftReferenceConfigurationPropertyCache<>(isImmutablePropertySource());
+		this.ancestorOfCheck = getAncestorOfCheck(mappers);
+	}
+
+	private BiPredicate<ConfigurationPropertyName, ConfigurationPropertyName> getAncestorOfCheck(
+			PropertyMapper[] mappers) {
+		BiPredicate<ConfigurationPropertyName, ConfigurationPropertyName> ancestorOfCheck = mappers[0]
+				.getAncestorOfCheck();
+		for (int i = 1; i < mappers.length; i++) {
+			ancestorOfCheck = ancestorOfCheck.or(mappers[i].getAncestorOfCheck());
+		}
+		return ancestorOfCheck;
 	}
 
 	private void assertEnumerablePropertySource() {
@@ -100,19 +114,35 @@ public ConfigurationProperty getConfigurationProperty(ConfigurationPropertyName
 
 	@Override
 	public Stream<ConfigurationPropertyName> stream() {
-		return getConfigurationPropertyNames().stream();
+		ConfigurationPropertyName[] names = getConfigurationPropertyNames();
+		return Arrays.stream(names).filter(Objects::nonNull);
 	}
 
 	@Override
 	public Iterator<ConfigurationPropertyName> iterator() {
-		return getConfigurationPropertyNames().iterator();
+		return new ConfigurationPropertyNamesIterator(getConfigurationPropertyNames());
+	}
+
+	@Override
+	public ConfigurationPropertyState containsDescendantOf(ConfigurationPropertyName name) {
+		ConfigurationPropertyState result = super.containsDescendantOf(name);
+		if (result != ConfigurationPropertyState.UNKNOWN) {
+			return result;
+		}
+		ConfigurationPropertyName[] candidates = getConfigurationPropertyNames();
+		for (ConfigurationPropertyName candidate : candidates) {
+			if (candidate != null && this.ancestorOfCheck.test(name, candidate)) {
+				return ConfigurationPropertyState.PRESENT;
+			}
+		}
+		return ConfigurationPropertyState.ABSENT;
 	}
 
-	private Collection<ConfigurationPropertyName> getConfigurationPropertyNames() {
+	private ConfigurationPropertyName[] getConfigurationPropertyNames() {
 		if (!isImmutablePropertySource()) {
 			return getMappings().getConfigurationPropertyNames(getPropertySource().getPropertyNames());
 		}
-		Collection<ConfigurationPropertyName> configurationPropertyNames = this.configurationPropertyNames;
+		ConfigurationPropertyName[] configurationPropertyNames = this.configurationPropertyNames;
 		if (configurationPropertyNames == null) {
 			configurationPropertyNames = getMappings()
 					.getConfigurationPropertyNames(getPropertySource().getPropertyNames());
@@ -121,24 +151,6 @@ private Collection<ConfigurationPropertyName> getConfigurationPropertyNames() {
 		return configurationPropertyNames;
 	}
 
-	@Override
-	public ConfigurationPropertyState containsDescendantOf(ConfigurationPropertyName name) {
-		ConfigurationPropertyState result = super.containsDescendantOf(name);
-		if (result == ConfigurationPropertyState.UNKNOWN) {
-			result = ConfigurationPropertyState.search(this, (candidate) -> isAncestorOf(name, candidate));
-		}
-		return result;
-	}
-
-	private boolean isAncestorOf(ConfigurationPropertyName name, ConfigurationPropertyName candidate) {
-		for (PropertyMapper mapper : getMappers()) {
-			if (mapper.isAncestorOf(name, candidate)) {
-				return true;
-			}
-		}
-		return false;
-	}
-
 	private Mappings getMappings() {
 		return this.cache.get(this::createMappings, this::updateMappings);
 	}
@@ -170,6 +182,8 @@ protected EnumerablePropertySource<?> getPropertySource() {
 
 	private static class Mappings {
 
+		private static final ConfigurationPropertyName[] EMPTY_NAMES_ARRAY = {};
+
 		private final PropertyMapper[] mappers;
 
 		private final boolean immutable;
@@ -178,7 +192,7 @@ private static class Mappings {
 
 		private volatile Map<String, ConfigurationPropertyName> reverseMappings;
 
-		private volatile Collection<ConfigurationPropertyName> configurationPropertyNames;
+		private volatile ConfigurationPropertyName[] configurationPropertyNames;
 
 		private volatile String[] lastUpdated;
 
@@ -230,30 +244,66 @@ private void updateMappings(String[] propertyNames) {
 			this.reverseMappings = reverseMappings;
 			this.lastUpdated = this.immutable ? null : propertyNames;
 			this.configurationPropertyNames = this.immutable
-					? Collections.unmodifiableCollection(reverseMappings.values()) : null;
+					? reverseMappings.values().toArray(new ConfigurationPropertyName[0]) : null;
 		}
 
 		List<String> getMapped(ConfigurationPropertyName configurationPropertyName) {
 			return this.mappings.getOrDefault(configurationPropertyName, Collections.emptyList());
 		}
 
-		Collection<ConfigurationPropertyName> getConfigurationPropertyNames(String[] propertyNames) {
-			Collection<ConfigurationPropertyName> names = this.configurationPropertyNames;
+		ConfigurationPropertyName[] getConfigurationPropertyNames(String[] propertyNames) {
+			ConfigurationPropertyName[] names = this.configurationPropertyNames;
 			if (names != null) {
 				return names;
 			}
 			Map<String, ConfigurationPropertyName> reverseMappings = this.reverseMappings;
 			if (reverseMappings == null || reverseMappings.isEmpty()) {
-				return Collections.emptySet();
+				return EMPTY_NAMES_ARRAY;
+			}
+			names = new ConfigurationPropertyName[propertyNames.length];
+			for (int i = 0; i < propertyNames.length; i++) {
+				names[i] = reverseMappings.get(propertyNames[i]);
 			}
-			List<ConfigurationPropertyName> relevantNames = new ArrayList<>(reverseMappings.size());
-			for (String propertyName : propertyNames) {
-				ConfigurationPropertyName configurationPropertyName = reverseMappings.get(propertyName);
-				if (configurationPropertyName != null) {
-					relevantNames.add(configurationPropertyName);
+			return names;
+		}
+
+	}
+
+	/**
+	 * ConfigurationPropertyNames iterator backed by an array.
+	 */
+	private static class ConfigurationPropertyNamesIterator implements Iterator<ConfigurationPropertyName> {
+
+		private final ConfigurationPropertyName[] names;
+
+		private int index = 0;
+
+		ConfigurationPropertyNamesIterator(ConfigurationPropertyName[] names) {
+			this.names = names;
+		}
+
+		@Override
+		public boolean hasNext() {
+			skipNulls();
+			return this.index < this.names.length;
+		}
+
+		@Override
+		public ConfigurationPropertyName next() {
+			skipNulls();
+			if (this.index >= this.names.length) {
+				throw new NoSuchElementException();
+			}
+			return this.names[this.index++];
+		}
+
+		private void skipNulls() {
+			while (this.index < this.names.length) {
+				if (this.names[this.index] != null) {
+					return;
 				}
+				this.index++;
 			}
-			return relevantNames;
 		}
 
 	}

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapper.java

@@ -20,6 +20,7 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
+import java.util.function.BiPredicate;
 
 import org.springframework.boot.context.properties.source.ConfigurationPropertyName.Form;
 
@@ -103,7 +104,11 @@ private static boolean isNumber(String string) {
 	}
 
 	@Override
-	public boolean isAncestorOf(ConfigurationPropertyName name, ConfigurationPropertyName candidate) {
+	public BiPredicate<ConfigurationPropertyName, ConfigurationPropertyName> getAncestorOfCheck() {
+		return this::isAncestorOf;
+	}
+
+	private boolean isAncestorOf(ConfigurationPropertyName name, ConfigurationPropertyName candidate) {
 		return name.isAncestorOf(candidate) || isLegacyAncestorOf(name, candidate);
 	}
 

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapperTests.java

@@ -16,6 +16,8 @@
 
 package org.springframework.boot.context.properties.source;
 
+import java.util.function.BiPredicate;
+
 import org.junit.jupiter.api.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -70,29 +72,28 @@ void underscoreWithWhitespaceShouldMapToEmptyString() {
 	@Test
 	void isAncestorOfConsidersLegacyNames() {
 		ConfigurationPropertyName name = ConfigurationPropertyName.of("my.spring-boot");
-		assertThat(getMapper().isAncestorOf(name, ConfigurationPropertyName.adapt("MY_SPRING_BOOT_PROPERTY", '_')))
-				.isTrue();
-		assertThat(getMapper().isAncestorOf(name, ConfigurationPropertyName.adapt("MY_SPRINGBOOT_PROPERTY", '_')))
-				.isTrue();
-		assertThat(getMapper().isAncestorOf(name, ConfigurationPropertyName.adapt("MY_BOOT_PROPERTY", '_'))).isFalse();
+		BiPredicate<ConfigurationPropertyName, ConfigurationPropertyName> check = getMapper().getAncestorOfCheck();
+		assertThat(check.test(name, ConfigurationPropertyName.adapt("MY_SPRING_BOOT_PROPERTY", '_'))).isTrue();
+		assertThat(check.test(name, ConfigurationPropertyName.adapt("MY_SPRINGBOOT_PROPERTY", '_'))).isTrue();
+		assertThat(check.test(name, ConfigurationPropertyName.adapt("MY_BOOT_PROPERTY", '_'))).isFalse();
 	}
 
 	@Test
 	void isAncestorOfWhenNonCanonicalSource() {
 		ConfigurationPropertyName name = ConfigurationPropertyName.adapt("my.springBoot", '.');
-		assertThat(getMapper().isAncestorOf(name, ConfigurationPropertyName.of("my.spring-boot.property"))).isTrue();
-		assertThat(getMapper().isAncestorOf(name, ConfigurationPropertyName.of("my.springboot.property"))).isTrue();
-		assertThat(getMapper().isAncestorOf(name, ConfigurationPropertyName.of("my.boot.property"))).isFalse();
+		BiPredicate<ConfigurationPropertyName, ConfigurationPropertyName> check = getMapper().getAncestorOfCheck();
+		assertThat(check.test(name, ConfigurationPropertyName.of("my.spring-boot.property"))).isTrue();
+		assertThat(check.test(name, ConfigurationPropertyName.of("my.springboot.property"))).isTrue();
+		assertThat(check.test(name, ConfigurationPropertyName.of("my.boot.property"))).isFalse();
 	}
 
 	@Test
 	void isAncestorOfWhenNonCanonicalAndDashedSource() {
 		ConfigurationPropertyName name = ConfigurationPropertyName.adapt("my.springBoot.input-value", '.');
-		assertThat(getMapper().isAncestorOf(name, ConfigurationPropertyName.of("my.spring-boot.input-value.property")))
-				.isTrue();
-		assertThat(getMapper().isAncestorOf(name, ConfigurationPropertyName.of("my.springboot.inputvalue.property")))
-				.isTrue();
-		assertThat(getMapper().isAncestorOf(name, ConfigurationPropertyName.of("my.boot.property"))).isFalse();
+		BiPredicate<ConfigurationPropertyName, ConfigurationPropertyName> check = getMapper().getAncestorOfCheck();
+		assertThat(check.test(name, ConfigurationPropertyName.of("my.spring-boot.input-value.property"))).isTrue();
+		assertThat(check.test(name, ConfigurationPropertyName.of("my.springboot.inputvalue.property"))).isTrue();
+		assertThat(check.test(name, ConfigurationPropertyName.of("my.boot.property"))).isFalse();
 	}
 
 }

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/TestPropertyMapper.java

@@ -53,9 +53,4 @@ public ConfigurationPropertyName map(String propertySourceName) {
 		return this.fromSource.getOrDefault(propertySourceName, ConfigurationPropertyName.EMPTY);
 	}
 
-	@Override
-	public boolean isAncestorOf(ConfigurationPropertyName name, ConfigurationPropertyName candidate) {
-		return name.isAncestorOf(candidate);
-	}
-
 }