Spring/spring-boot
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Improve ConfigurationPropertySource performance

Browse Source 
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
This commit is contained in:
Phillip Webb
2020-05-12 13:09:11 -07:00
parent 376098d080
commit 4af6e7ff99
7 changed files with 119 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -79,11 +79,6 @@ final class DefaultPropertyMapper implements PropertyMapper {
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;

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

@@ -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 @@ import org.springframework.core.env.PropertySource;
*/
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;
}
}

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

@@ -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;
}

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

@@ -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 @@ import org.springframework.util.MultiValueMap;
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 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
@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());
}
private Collection<ConfigurationPropertyName> 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 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 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
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 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
private static class Mappings {
private static final ConfigurationPropertyName[] EMPTY_NAMES_ARRAY = {};
private final PropertyMapper[] mappers;
private final boolean immutable;
@@ -178,7 +192,7 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
private volatile Map<String, ConfigurationPropertyName> reverseMappings;
private volatile Collection<ConfigurationPropertyName> configurationPropertyNames;
private volatile ConfigurationPropertyName[] configurationPropertyNames;
private volatile String[] lastUpdated;
@@ -230,30 +244,66 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
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;
}
List<ConfigurationPropertyName> relevantNames = new ArrayList<>(reverseMappings.size());
for (String propertyName : propertyNames) {
ConfigurationPropertyName configurationPropertyName = reverseMappings.get(propertyName);
if (configurationPropertyName != null) {
relevantNames.add(configurationPropertyName);
names = new ConfigurationPropertyName[propertyNames.length];
for (int i = 0; i < propertyNames.length; i++) {
names[i] = reverseMappings.get(propertyNames[i]);
}
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;
}
}

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

@@ -20,6 +20,7 @@ import java.util.Arrays;
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 @@ final class SystemEnvironmentPropertyMapper implements PropertyMapper {
}
@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);
}

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

@@ -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 @@ class SystemEnvironmentPropertyMapperTests extends AbstractPropertyMapperTests {
@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();
}
}

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

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