Commit 0607af87 authored by Phillip Webb's avatar Phillip Webb

Improve ConfigurationPropertySource performance

Further improve the performance of `containsDescendantOf` by using
a Map to limit the number of candidates that need checking.

Closes gh-21416
parent 4af6e7ff
...@@ -194,6 +194,16 @@ public final class ConfigurationPropertyName implements Comparable<Configuration ...@@ -194,6 +194,16 @@ public final class ConfigurationPropertyName implements Comparable<Configuration
return new ConfigurationPropertyName(this.elements.append(additionalElements)); return new ConfigurationPropertyName(this.elements.append(additionalElements));
} }
/**
* Return the parent of this {@link ConfigurationPropertyName} or
* {@link ConfigurationPropertyName#EMPTY} if there is no parent.
* @return the parent name
*/
public ConfigurationPropertyName getParent() {
int numberOfElements = getNumberOfElements();
return (numberOfElements <= 1) ? EMPTY : chop(numberOfElements - 1);
}
/** /**
* Return a new {@link ConfigurationPropertyName} by chopping this name to the given * Return a new {@link ConfigurationPropertyName} by chopping this name to the given
* {@code size}. For example, {@code chop(1)} on the name {@code foo.bar} will return * {@code size}. For example, {@code chop(1)} on the name {@code foo.bar} will return
......
...@@ -20,11 +20,12 @@ import java.util.Arrays; ...@@ -20,11 +20,12 @@ import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.ConcurrentModificationException; import java.util.ConcurrentModificationException;
import java.util.HashMap; import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator; import java.util.Iterator;
import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.NoSuchElementException; import java.util.NoSuchElementException;
import java.util.Objects; import java.util.Objects;
import java.util.Set;
import java.util.function.BiPredicate; import java.util.function.BiPredicate;
import java.util.function.Supplier; import java.util.function.Supplier;
import java.util.stream.Stream; import java.util.stream.Stream;
...@@ -37,8 +38,6 @@ import org.springframework.core.env.MapPropertySource; ...@@ -37,8 +38,6 @@ import org.springframework.core.env.MapPropertySource;
import org.springframework.core.env.PropertySource; import org.springframework.core.env.PropertySource;
import org.springframework.core.env.StandardEnvironment; import org.springframework.core.env.StandardEnvironment;
import org.springframework.core.env.SystemEnvironmentPropertySource; import org.springframework.core.env.SystemEnvironmentPropertySource;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
/** /**
* {@link ConfigurationPropertySource} backed by an {@link EnumerablePropertySource}. * {@link ConfigurationPropertySource} backed by an {@link EnumerablePropertySource}.
...@@ -54,17 +53,17 @@ import org.springframework.util.MultiValueMap; ...@@ -54,17 +53,17 @@ import org.springframework.util.MultiValueMap;
class SpringIterableConfigurationPropertySource extends SpringConfigurationPropertySource class SpringIterableConfigurationPropertySource extends SpringConfigurationPropertySource
implements IterableConfigurationPropertySource, CachingConfigurationPropertySource { implements IterableConfigurationPropertySource, CachingConfigurationPropertySource {
private volatile ConfigurationPropertyName[] configurationPropertyNames; private final BiPredicate<ConfigurationPropertyName, ConfigurationPropertyName> ancestorOfCheck;
private final SoftReferenceConfigurationPropertyCache<Mappings> cache; private final SoftReferenceConfigurationPropertyCache<Mappings> cache;
private final BiPredicate<ConfigurationPropertyName, ConfigurationPropertyName> ancestorOfCheck; private volatile ConfigurationPropertyName[] configurationPropertyNames;
SpringIterableConfigurationPropertySource(EnumerablePropertySource<?> propertySource, PropertyMapper... mappers) { SpringIterableConfigurationPropertySource(EnumerablePropertySource<?> propertySource, PropertyMapper... mappers) {
super(propertySource, mappers); super(propertySource, mappers);
assertEnumerablePropertySource(); assertEnumerablePropertySource();
this.cache = new SoftReferenceConfigurationPropertyCache<>(isImmutablePropertySource());
this.ancestorOfCheck = getAncestorOfCheck(mappers); this.ancestorOfCheck = getAncestorOfCheck(mappers);
this.cache = new SoftReferenceConfigurationPropertyCache<>(isImmutablePropertySource());
} }
private BiPredicate<ConfigurationPropertyName, ConfigurationPropertyName> getAncestorOfCheck( private BiPredicate<ConfigurationPropertyName, ConfigurationPropertyName> getAncestorOfCheck(
...@@ -129,6 +128,9 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope ...@@ -129,6 +128,9 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
if (result != ConfigurationPropertyState.UNKNOWN) { if (result != ConfigurationPropertyState.UNKNOWN) {
return result; return result;
} }
if (this.ancestorOfCheck == PropertyMapper.DEFAULT_ANCESTOR_OF_CHECK) {
return getMappings().containsDescendantOf(name, this.ancestorOfCheck);
}
ConfigurationPropertyName[] candidates = getConfigurationPropertyNames(); ConfigurationPropertyName[] candidates = getConfigurationPropertyNames();
for (ConfigurationPropertyName candidate : candidates) { for (ConfigurationPropertyName candidate : candidates) {
if (candidate != null && this.ancestorOfCheck.test(name, candidate)) { if (candidate != null && this.ancestorOfCheck.test(name, candidate)) {
...@@ -156,7 +158,8 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope ...@@ -156,7 +158,8 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
} }
private Mappings createMappings() { private Mappings createMappings() {
return new Mappings(getMappers(), isImmutablePropertySource()); return new Mappings(getMappers(), isImmutablePropertySource(),
this.ancestorOfCheck == PropertyMapper.DEFAULT_ANCESTOR_OF_CHECK);
} }
private Mappings updateMappings(Mappings mappings) { private Mappings updateMappings(Mappings mappings) {
...@@ -188,17 +191,22 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope ...@@ -188,17 +191,22 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
private final boolean immutable; private final boolean immutable;
private volatile MultiValueMap<ConfigurationPropertyName, String> mappings; private final boolean trackDescendants;
private volatile Map<ConfigurationPropertyName, Set<String>> mappings;
private volatile Map<String, ConfigurationPropertyName> reverseMappings; private volatile Map<String, ConfigurationPropertyName> reverseMappings;
private volatile Map<ConfigurationPropertyName, Set<ConfigurationPropertyName>> descendants;
private volatile ConfigurationPropertyName[] configurationPropertyNames; private volatile ConfigurationPropertyName[] configurationPropertyNames;
private volatile String[] lastUpdated; private volatile String[] lastUpdated;
Mappings(PropertyMapper[] mappers, boolean immutable) { Mappings(PropertyMapper[] mappers, boolean immutable, boolean trackDescendants) {
this.mappers = mappers; this.mappers = mappers;
this.immutable = immutable; this.immutable = immutable;
this.trackDescendants = trackDescendants;
} }
void updateMappings(Supplier<String[]> propertyNames) { void updateMappings(Supplier<String[]> propertyNames) {
...@@ -223,32 +231,52 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope ...@@ -223,32 +231,52 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
if (lastUpdated != null && Arrays.equals(lastUpdated, propertyNames)) { if (lastUpdated != null && Arrays.equals(lastUpdated, propertyNames)) {
return; return;
} }
MultiValueMap<ConfigurationPropertyName, String> previousMappings = this.mappings; int size = propertyNames.length;
MultiValueMap<ConfigurationPropertyName, String> mappings = (previousMappings != null) Map<ConfigurationPropertyName, Set<String>> mappings = cloneOrCreate(this.mappings, size);
? new LinkedMultiValueMap<>(previousMappings) : new LinkedMultiValueMap<>(propertyNames.length); Map<String, ConfigurationPropertyName> reverseMappings = cloneOrCreate(this.reverseMappings, size);
Map<String, ConfigurationPropertyName> previousReverseMappings = this.reverseMappings; Map<ConfigurationPropertyName, Set<ConfigurationPropertyName>> descendants = cloneOrCreate(this.descendants,
Map<String, ConfigurationPropertyName> reverseMappings = (previousReverseMappings != null) size);
? new HashMap<>(previousReverseMappings) : new HashMap<>(propertyNames.length);
for (PropertyMapper propertyMapper : this.mappers) { for (PropertyMapper propertyMapper : this.mappers) {
for (String propertyName : propertyNames) { for (String propertyName : propertyNames) {
if (!reverseMappings.containsKey(propertyName)) { if (!reverseMappings.containsKey(propertyName)) {
ConfigurationPropertyName configurationPropertyName = propertyMapper.map(propertyName); ConfigurationPropertyName configurationPropertyName = propertyMapper.map(propertyName);
if (configurationPropertyName != null && !configurationPropertyName.isEmpty()) { if (configurationPropertyName != null && !configurationPropertyName.isEmpty()) {
mappings.add(configurationPropertyName, propertyName); add(mappings, configurationPropertyName, propertyName);
reverseMappings.put(propertyName, configurationPropertyName); reverseMappings.put(propertyName, configurationPropertyName);
if (this.trackDescendants) {
addParents(descendants, configurationPropertyName);
}
} }
} }
} }
} }
this.mappings = mappings; this.mappings = mappings;
this.reverseMappings = reverseMappings; this.reverseMappings = reverseMappings;
this.descendants = descendants;
this.lastUpdated = this.immutable ? null : propertyNames; this.lastUpdated = this.immutable ? null : propertyNames;
this.configurationPropertyNames = this.immutable this.configurationPropertyNames = this.immutable
? reverseMappings.values().toArray(new ConfigurationPropertyName[0]) : null; ? reverseMappings.values().toArray(new ConfigurationPropertyName[0]) : null;
} }
List<String> getMapped(ConfigurationPropertyName configurationPropertyName) { private <K, V> Map<K, V> cloneOrCreate(Map<K, V> source, int size) {
return this.mappings.getOrDefault(configurationPropertyName, Collections.emptyList()); return (source != null) ? new HashMap<>(source) : new HashMap<>(size);
}
private void addParents(Map<ConfigurationPropertyName, Set<ConfigurationPropertyName>> descendants,
ConfigurationPropertyName name) {
ConfigurationPropertyName parent = name;
while (!parent.isEmpty()) {
add(descendants, parent, name);
parent = parent.getParent();
}
}
private <K, T> void add(Map<K, Set<T>> map, K key, T value) {
map.computeIfAbsent(key, (k) -> new HashSet<>()).add(value);
}
Set<String> getMapped(ConfigurationPropertyName configurationPropertyName) {
return this.mappings.getOrDefault(configurationPropertyName, Collections.emptySet());
} }
ConfigurationPropertyName[] getConfigurationPropertyNames(String[] propertyNames) { ConfigurationPropertyName[] getConfigurationPropertyNames(String[] propertyNames) {
...@@ -267,6 +295,20 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope ...@@ -267,6 +295,20 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
return names; return names;
} }
ConfigurationPropertyState containsDescendantOf(ConfigurationPropertyName name,
BiPredicate<ConfigurationPropertyName, ConfigurationPropertyName> ancestorOfCheck) {
if (name.isEmpty() && !this.descendants.isEmpty()) {
return ConfigurationPropertyState.PRESENT;
}
Set<ConfigurationPropertyName> candidates = this.descendants.getOrDefault(name, Collections.emptySet());
for (ConfigurationPropertyName candidate : candidates) {
if (ancestorOfCheck.test(name, candidate)) {
return ConfigurationPropertyState.PRESENT;
}
}
return ConfigurationPropertyState.ABSENT;
}
} }
/** /**
......
...@@ -414,6 +414,26 @@ class ConfigurationPropertyNameTests { ...@@ -414,6 +414,26 @@ class ConfigurationPropertyNameTests {
assertThat((Object) name.append(null)).isSameAs(name); assertThat((Object) name.append(null)).isSameAs(name);
} }
@Test
void getParentShouldReturnParent() {
ConfigurationPropertyName name = ConfigurationPropertyName.of("this.is.a.multipart.name");
ConfigurationPropertyName p1 = name.getParent();
ConfigurationPropertyName p2 = p1.getParent();
ConfigurationPropertyName p3 = p2.getParent();
ConfigurationPropertyName p4 = p3.getParent();
ConfigurationPropertyName p5 = p4.getParent();
assertThat(p1).hasToString("this.is.a.multipart");
assertThat(p2).hasToString("this.is.a");
assertThat(p3).hasToString("this.is");
assertThat(p4).hasToString("this");
assertThat(p5).isEqualTo(ConfigurationPropertyName.EMPTY);
}
@Test
void getParentWhenEmptyShouldReturnEmpty() {
assertThat(ConfigurationPropertyName.EMPTY.getParent()).isEqualTo(ConfigurationPropertyName.EMPTY);
}
@Test @Test
void chopWhenLessThenSizeShouldReturnChopped() { void chopWhenLessThenSizeShouldReturnChopped() {
ConfigurationPropertyName name = ConfigurationPropertyName.of("foo.bar.baz"); ConfigurationPropertyName name = ConfigurationPropertyName.of("foo.bar.baz");
...@@ -567,6 +587,15 @@ class ConfigurationPropertyNameTests { ...@@ -567,6 +587,15 @@ class ConfigurationPropertyNameTests {
assertThat((Object) n14).isNotEqualTo(n15); assertThat((Object) n14).isNotEqualTo(n15);
} }
@Test
void equalsAndHashCodeAfterOperations() {
ConfigurationPropertyName n1 = ConfigurationPropertyName.of("nested");
ConfigurationPropertyName n2 = ConfigurationPropertyName.EMPTY.append("nested");
ConfigurationPropertyName n3 = ConfigurationPropertyName.of("nested.value").getParent();
assertThat(n1.hashCode()).isEqualTo(n2.hashCode()).isEqualTo(n3.hashCode());
assertThat(n1).isEqualTo(n2).isEqualTo(n3);
}
@Test @Test
void equalsWhenStartsWith() { void equalsWhenStartsWith() {
// gh-14665 // gh-14665
......
...@@ -143,6 +143,21 @@ class ConfigurationPropertySourcesTests { ...@@ -143,6 +143,21 @@ class ConfigurationPropertySourcesTests {
testPropertySourcePerformance(false, 5000); testPropertySourcePerformance(false, 5000);
} }
@Test // gh-21416
void decendantOfPropertyAccessWhenMutableWithCacheShouldBePerformant() {
StandardEnvironment environment = createPerformanceTestEnvironment(true);
Iterable<ConfigurationPropertySource> sources = ConfigurationPropertySources.get(environment);
ConfigurationPropertyName missing = ConfigurationPropertyName.of("missing");
long start = System.nanoTime();
for (int i = 0; i < 1000; i++) {
for (ConfigurationPropertySource source : sources) {
assertThat(source.containsDescendantOf(missing)).isEqualTo(ConfigurationPropertyState.ABSENT);
}
}
long total = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
assertThat(total).isLessThan(1000);
}
private void testPropertySourcePerformance(boolean immutable, int maxTime) { private void testPropertySourcePerformance(boolean immutable, int maxTime) {
StandardEnvironment environment = createPerformanceTestEnvironment(immutable); StandardEnvironment environment = createPerformanceTestEnvironment(immutable);
testPropertySourcePerformance(environment, maxTime); testPropertySourcePerformance(environment, maxTime);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment