Provide a possibility to extend/override configuration properties binding process

[dmitriytsokolov]

We faced an issue where a property source for 700+- objects (consists of 100+- fields) binds to java objects slowly: around 6 minutes. Our services rely on spring cloud hot refresh and because of slow binding users are struggling. We could optimize this process for our use case, but most of the binding classes are closed for extension.

[philwebb]

There's not really enough information here for us to be able to take any action. Could you describe what optimizations you're trying to apply and what changes you'd need to do them?

A sample application that shows the issue would also be very helpful. Perhaps if we profile it there might be some general optimizations we can apply for everyone.

[dmitriytsokolov]

I'm mostly interested in extending org.springframework.boot.context.properties.bind.Binder class, but almost all classes inside org.springframework.boot.context.properties.bind are package-private.

About optimisation. I'm not sure it would work generally, but the idea is the following: I have an ConfigurationProperties class which first child field is a hashmap, so my yml files that are bond to ConfigProps looks the following way:

config-props-prefix:
  some-root-field-name:
    map-key-1:
      obj-field1-key: value1
      obj-field2-key: value2
      ...

And I have thousands of such files that are bond to the map. And I don't need a property config-props-prefix.some-root-field-name.${some-map-key}.obj-field1-key to be processed by binder multiple times, so I could cache the result and use it for binding process of other map entries. Does it make any sense to you?

I'm using spring boot 2.7.18, but I've tested on spring boot 3.2.7 and performance is the same.

[wilkinsona]

@dmitriytsokolov thanks for the additional details. Can you please share a sample application that uses Spring Boot 3.2.x or later that demonstrates what you've described above?

[dmitriytsokolov]

@wilkinsona sure. Should I just create an example with couple of config files, or with hundreds to emulate performance issues?

[wilkinsona]

Thanks. Something that replicates the performance issue would be ideal. Perhaps you could use a script to generate a sufficient volume of synthetic test data?

[dmitriytsokolov]

attaching example of spring app with 1000 configs that is started up for 40-50s locally. In my real app I have more fields and more spring placeholders, so app start time/hot refresh takes 5-6 minutes demo_config_props.zip

One more thing: initially I though that I could cache results of resolved spring placeholders (because I have a lot of them) and get some performance benefits, so I've made the following change in Binder class locally:

    private <T> Object bindProperty(Bindable<T> target, Context context, ConfigurationProperty property) {
        context.setConfigurationProperty(property);
        Object original = property.getValue();
        Object fromCache = this.resolvedPlaceholdersCache.get(original);
        if (fromCache != null) {
            return fromCache;
        }
        Object result = this.placeholdersResolver.resolvePlaceholders(original);
        result = context.getConverter().convert(result, target);
        this.resolvedPlaceholdersCache.put(original, result);
        return result;
    }

but I've got approximately minus 20 seconds out of 5-6 minutes of binding time. It's not much, but still with the ability to override/extend some binding logic I could reduce it even more knowing some specifics of my serivce

Anyway, will be waiting for your input, thanks a lot

[dmitriytsokolov]

The other option I'm thinking of: As you see I have a simple hashmap on the root level. So if only a part of the binding beans were opened for extension, I could try rebinding config props partially manually (binding of each map's element asynchronously and build the map by myself). This should help, but of course I'm not sure about it. I'm going to install local version of spring with required updates and will try to implement this approach in my app. But I would appreciate your input on this idea.

[Mobe91]

I am experiencing similar issues with a spring-boot application that I am working on. It is a multi-tenant application where configuration for each tenant is provided via environment variables (~6000 keys in total atm). The binding of these properties takes more than 10 minutes. This happens on startup and whenever the configuration is refreshed using Spring Cloud as suggested by @dmitriytsokolov. I also suspect this to be the cause for the issue I commented on here (at least in my case).

I have done some profiling for this and was able to identify the following performance hotspots:

1. Most of the time was spent in org.springframework.boot.context.properties.source.SystemEnvironmentPropertyMapper#isLegacyAncestorOf. I am wondering if this behavior is still needed or if it could be removed.

   It was hard for me to work around this issue as the property mappers are statically constructed and assigned in org.springframework.boot.context.properties.source.SpringConfigurationPropertySource#from. I ended up creating a copy of SpringConfigurationPropertySource and SpringIterableConfigurationPropertySource that use a modified SystemEnvironmentPropertyMapper (stripped the legacy stuff from it).

2. Also org.springframework.core.env.MapPropertySource#getPropertyNames was a hotspot as it seems to re-construct the property names many times during binding.

   I was able to work around this by wrapping all property sources in an immutable wrapper before providing them to the binder. The wrapper evaluates the property names only once at construction time.

3. Another hotspot was org.springframework.boot.context.properties.source.SpringIterableConfigurationPropertySource.Mappings#updateMappings(java.lang.String[]) which was invoked very frequently. Most of the ConfigurationPropertySources were not immutable so the Mappings were updated every time they were accessed.

   I reused the workaround from (2) to solve this by implementing org.springframework.boot.origin.OriginLookup and returning true for org.springframework.boot.origin.OriginLookup#isImmutable. At that point the binding runtime was down to < 10 seconds.

4. The last hotspot was in org.springframework.boot.context.properties.source.SpringIterableConfigurationPropertySource#containsDescendantOf. Here the descendent relationship cache implemented in org.springframework.boot.context.properties.source.SpringIterableConfigurationPropertySource.Mappings is only used if this.ancestorOfCheck == PropertyMapper.DEFAULT_ANCESTOR_OF_CHECK which is not the case for org.springframework.core.env.SystemEnvironmentPropertySource. So the descendent caching is not in effect for the SystemEnvironmentPropertySource.

   I decided to not create a workaround for this as it would have required further modification to the Spring implementation and the performance was at an acceptable level already. However, I assume that there would be a further substantial reduction in runtime with an improved descendent caching.

Above evaluation was done using Spring Boot 3.1.6. However, I checked the latest Spring Boot sources and there seems to have been no relevant changes to the code sections mentioned above.

[philwebb]

Unfortunately the relaxed binding code continues to be a source of frustration. @Mobe91 have you tried using the ConfigurationPropertyCaching interface to enable caching globally?

[Mobe91]

Thank you for the helpful pointer @philwebb , I did not know about the ConfigurationPropertyCaching interface!

By enabling caching globally using the ConfigurationPropertyCaching interface I was actually able to remove the workarounds for (2) and (3) mentioned in my earlier comment without taking a performance hit.

Unfortunately, the workaround for (1) is still needed to yield acceptable performance due to the lack of descendent caching for SystemEnvironmentPropertySource mentioned in (4).

Some numbers:

• With workaround (1): ~8 seconds
• Without workaround (1): ~55 seconds

[quaff]

Unfortunately the relaxed binding code continues to be a source of frustration.

Could it be disabled by default since next major release?

[dmitriytsokolov]

Hi @Mobe91. Thanks for the comment. Would it be possible for you to somehow provide snippets of your workarounds?

@philwebb is anyone going to take a look at the example I provided? Would you suggest anything on this matter?

[wilkinsona]

@dmitriytsokolov We're a small team with many different priorities. Rest assured, we will look at the sample when we have time to do so. That may take a little while as we work on higher priority issues for 3.4.0-RC1 that releases next week. Thank you for your patience in the meantime.

[dmitriytsokolov]

Of course, thanks a lot @wilkinsona!

[Mobe91]

@dmitriytsokolov

Would it be possible for you to somehow provide snippets of your workarounds?

Sure, this is what it comes down to:

/**
 * Mostly a copy of {@link SpringConfigurationPropertySource} to pass different {@link PropertyMapper}s to the
 * constructed {@link ConfigurationPropertySource}. Sadly, at the time of writing Spring does not provide a way
 * to do this differently.
 */
public class CustomConfigurationPropertySourceFactory {

    private static final PropertyMapper[] DEFAULT_MAPPERS = { DefaultPropertyMapper.INSTANCE };

    private static final PropertyMapper[] SYSTEM_ENVIRONMENT_MAPPERS = {
        NonLegacySystemEnvironmentPropertyMapper.INSTANCE,
        DefaultPropertyMapper.INSTANCE };

    public static ConfigurationPropertySource from(PropertySource<?> source) {
        Assert.notNull(source, "Source must not be null");
        PropertyMapper[] mappers = getPropertyMappers(source);
        if (isFullEnumerable(source)) {
            return new SpringIterableConfigurationPropertySource((EnumerablePropertySource<?>) source,
                mappers);
        }
        return new SpringConfigurationPropertySource(source, mappers);
    }

    // need to copy some more static methods here - skipped for brevity
}

/**
 * Copy of {@link SystemEnvironmentPropertyMapper} modified to remove the handling of legacy property naming
 * <a href="https://github.com/spring-projects/spring-boot/issues/42361#issuecomment-2399941504">for performance
 * reasons</a>.
 */
class NonLegacySystemEnvironmentPropertyMapper implements PropertyMapper {

    // Get rid of isLegacyAncestorOf and all uses of it
}

Use it:

        // Make a copy of the environment's PropertySources to wrap them in ConfigurationPropertySources created
        // using our CustomConfigurationPropertySourceFactory.
        Iterable<ConfigurationPropertySource> configurationPropertySources = env.getPropertySources().stream()
            .filter(Predicate.not(ConfigurationPropertySources::isAttachedConfigurationPropertySource))
            .map(CustomConfigurationPropertySourceFactory::from)
            .toList();
        // Enabling caching for the copied ConfigurationPropertySources at this point is important for the binding
        // performance.
        ConfigurationPropertyCaching.get(configurationPropertySources).enable();
        Binder binder = new Binder(configurationPropertySources, new PropertySourcesPlaceholdersResolver(env));