From a04c136dbd865b2d55cfe56640cdcd933e1f0099 Mon Sep 17 00:00:00 2001 From: Roberto Cortez Date: Tue, 12 Nov 2024 16:02:46 +0000 Subject: [PATCH] Reduce allocation in matching of unknown properties --- .../BuildTimeConfigurationReader.java | 33 ++--- .../RunTimeConfigurationGenerator.java | 110 +++++++---------- .../runtime/configuration/PropertiesUtil.java | 116 ++++-------------- 3 files changed, 87 insertions(+), 172 deletions(-) diff --git a/core/deployment/src/main/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReader.java b/core/deployment/src/main/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReader.java index 0903a5b88b568..d5e5bf8e364c9 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReader.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReader.java @@ -80,6 +80,7 @@ import io.smallrye.config.EnvConfigSource; import io.smallrye.config.ProfileConfigSourceInterceptor; import io.smallrye.config.PropertiesConfigSource; +import io.smallrye.config.PropertyName; import io.smallrye.config.SecretKeys; import io.smallrye.config.SmallRyeConfig; import io.smallrye.config.SmallRyeConfigBuilder; @@ -626,22 +627,19 @@ ReadResult run() { objectsByClass.put(mapping.getKlass(), config.getConfigMapping(mapping.getKlass(), mapping.getPrefix())); } - // Build Time Values Recording - for (ConfigClass mapping : buildTimeMappings) { - Set mappedProperties = ConfigMappings.mappedProperties(mapping, allProperties); - for (String property : mappedProperties) { + Set buildTimeNames = getMappingsNames(buildTimeMappings); + Set buildTimeRunTimeNames = getMappingsNames(buildTimeRunTimeMappings); + Set runTimeNames = getMappingsNames(runTimeMappings); + for (String property : allProperties) { + PropertyName name = new PropertyName(property); + if (buildTimeNames.contains(name)) { unknownBuildProperties.remove(property); ConfigValue value = config.getConfigValue(property); if (value.getRawValue() != null) { allBuildTimeValues.put(value.getNameProfiled(), value.getRawValue()); } } - } - - // Build Time and Run Time Values Recording - for (ConfigClass mapping : buildTimeRunTimeMappings) { - Set mappedProperties = ConfigMappings.mappedProperties(mapping, allProperties); - for (String property : mappedProperties) { + if (buildTimeRunTimeNames.contains(name)) { unknownBuildProperties.remove(property); ConfigValue value = config.getConfigValue(property); if (value.getRawValue() != null) { @@ -649,12 +647,7 @@ ReadResult run() { buildTimeRunTimeValues.put(value.getNameProfiled(), value.getRawValue()); } } - } - - // Run Time Values Recording - for (ConfigClass mapping : runTimeMappings) { - Set mappedProperties = ConfigMappings.mappedProperties(mapping, allProperties); - for (String property : mappedProperties) { + if (runTimeNames.contains(name)) { unknownBuildProperties.remove(property); ConfigValue value = runtimeConfig.getConfigValue(property); if (value.getRawValue() != null) { @@ -1216,6 +1209,14 @@ private static void getDefaults( patternMap.getChild(childName)); } } + + private static Set getMappingsNames(final List configMappings) { + Set names = new HashSet<>(); + for (ConfigClass configMapping : configMappings) { + names.addAll(ConfigMappings.getProperties(configMapping).keySet()); + } + return PropertiesUtil.toPropertyNames(names); + } } public static final class ReadResult { diff --git a/core/deployment/src/main/java/io/quarkus/deployment/configuration/RunTimeConfigurationGenerator.java b/core/deployment/src/main/java/io/quarkus/deployment/configuration/RunTimeConfigurationGenerator.java index e82e137f8dd01..0d8e72863855b 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/configuration/RunTimeConfigurationGenerator.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/configuration/RunTimeConfigurationGenerator.java @@ -66,7 +66,7 @@ import io.smallrye.config.ConfigMappings; import io.smallrye.config.ConfigMappings.ConfigClass; import io.smallrye.config.Converters; -import io.smallrye.config.KeyMap; +import io.smallrye.config.PropertyName; import io.smallrye.config.SmallRyeConfig; import io.smallrye.config.SmallRyeConfigBuilder; @@ -86,10 +86,13 @@ public final class RunTimeConfigurationGenerator { public static final MethodDescriptor REINIT = MethodDescriptor.ofMethod(CONFIG_CLASS_NAME, "reinit", void.class); public static final MethodDescriptor C_READ_CONFIG = MethodDescriptor.ofMethod(CONFIG_CLASS_NAME, "readConfig", void.class); + + static final FieldDescriptor C_MAPPED_PROPERTIES = FieldDescriptor.of(CONFIG_CLASS_NAME, "mappedProperties", Set.class); + static final MethodDescriptor C_GENERATE_MAPPED_PROPERTIES = MethodDescriptor.ofMethod(CONFIG_CLASS_NAME, + "generateMappedProperties", Set.class); + static final MethodDescriptor PN_NEW = MethodDescriptor.ofConstructor(PropertyName.class, String.class); static final FieldDescriptor C_UNKNOWN = FieldDescriptor.of(CONFIG_CLASS_NAME, "unknown", Set.class); static final FieldDescriptor C_UNKNOWN_RUNTIME = FieldDescriptor.of(CONFIG_CLASS_NAME, "unknownRuntime", Set.class); - static final MethodDescriptor C_MAPPED_PROPERTIES = MethodDescriptor.ofMethod(CONFIG_CLASS_NAME, "mappedProperties", - KeyMap.class); static final MethodDescriptor CD_INVALID_VALUE = MethodDescriptor.ofMethod(ConfigDiagnostic.class, "invalidValue", void.class, String.class, IllegalArgumentException.class); @@ -177,7 +180,6 @@ public final class RunTimeConfigurationGenerator { Object.class, String.class, Converter.class); static final MethodDescriptor SRCB_NEW = MethodDescriptor.ofConstructor(SmallRyeConfigBuilder.class); - static final MethodDescriptor SRCB_WITH_CONVERTER = MethodDescriptor.ofMethod(SmallRyeConfigBuilder.class, "withConverter", ConfigBuilder.class, Class.class, int.class, Converter.class); static final MethodDescriptor SRCB_WITH_CUSTOMIZER = MethodDescriptor.ofMethod(AbstractConfigBuilder.class, @@ -185,15 +187,14 @@ public final class RunTimeConfigurationGenerator { static final MethodDescriptor SRCB_BUILD = MethodDescriptor.ofMethod(SmallRyeConfigBuilder.class, "build", SmallRyeConfig.class); - static final MethodDescriptor PU_FILTER_PROPERTIES_IN_ROOTS = MethodDescriptor.ofMethod(PropertiesUtil.class, - "filterPropertiesInRoots", Iterable.class, Iterable.class, Set.class); - static final MethodDescriptor PU_IS_PROPERTY_QUARKUS_COMPOUND_NAME = MethodDescriptor.ofMethod(PropertiesUtil.class, "isPropertyQuarkusCompoundName", boolean.class, NameIterator.class); - static final MethodDescriptor PU_FILTER_UNKNOWN = MethodDescriptor.ofMethod(PropertiesUtil.class, "filterUnknown", - void.class, Set.class, KeyMap.class); + static final MethodDescriptor PU_IS_PROPERTY_IN_ROOTS = MethodDescriptor.ofMethod(PropertiesUtil.class, "isPropertyInRoots", + boolean.class, String.class, Set.class); static final MethodDescriptor HS_NEW = MethodDescriptor.ofConstructor(HashSet.class); static final MethodDescriptor HS_ADD = MethodDescriptor.ofMethod(HashSet.class, "add", boolean.class, Object.class); + static final MethodDescriptor HS_CONTAINS = MethodDescriptor.ofMethod(HashSet.class, "contains", boolean.class, + Object.class); // todo: more space-efficient sorted map impl static final MethodDescriptor TM_NEW = MethodDescriptor.ofConstructor(TreeMap.class); @@ -261,8 +262,8 @@ public static final class GenerateOperation implements AutoCloseable { roots = Assert.checkNotNullParam("builder.roots", builder.getBuildTimeReadResult().getAllRoots()); additionalTypes = Assert.checkNotNullParam("additionalTypes", builder.getAdditionalTypes()); cc = ClassCreator.builder().classOutput(classOutput).className(CONFIG_CLASS_NAME).setFinal(true).build(); + generateMappedProperties(); generateEmptyParsers(); - generateUnknownFilter(); // not instantiable try (MethodCreator mc = cc.getMethodCreator(MethodDescriptor.ofConstructor(CONFIG_CLASS_NAME))) { mc.setModifiers(Opcodes.ACC_PRIVATE); @@ -280,10 +281,13 @@ public static final class GenerateOperation implements AutoCloseable { clinit = cc.getMethodCreator(MethodDescriptor.ofMethod(CONFIG_CLASS_NAME, "", void.class)); clinit.setModifiers(Opcodes.ACC_STATIC); - cc.getFieldCreator(C_UNKNOWN).setModifiers(Opcodes.ACC_STATIC | Opcodes.ACC_FINAL); + cc.getFieldCreator(C_MAPPED_PROPERTIES).setModifiers(Opcodes.ACC_STATIC); + clinit.writeStaticField(C_MAPPED_PROPERTIES, clinit.invokeStaticMethod(C_GENERATE_MAPPED_PROPERTIES)); + + cc.getFieldCreator(C_UNKNOWN).setModifiers(Opcodes.ACC_STATIC); clinit.writeStaticField(C_UNKNOWN, clinit.newInstance(HS_NEW)); - cc.getFieldCreator(C_UNKNOWN_RUNTIME).setModifiers(Opcodes.ACC_STATIC | Opcodes.ACC_FINAL); + cc.getFieldCreator(C_UNKNOWN_RUNTIME).setModifiers(Opcodes.ACC_STATIC); clinit.writeStaticField(C_UNKNOWN_RUNTIME, clinit.newInstance(HS_NEW)); clinitNameBuilder = clinit.newInstance(SB_NEW); @@ -457,10 +461,6 @@ public void run() { // generate sweep for clinit configSweepLoop(siParserBody, clinit, clinitConfig, getRegisteredRoots(BUILD_AND_RUN_TIME_FIXED), Type.BUILD_TIME); - - clinit.invokeStaticMethod(PU_FILTER_UNKNOWN, - clinit.readStaticField(C_UNKNOWN), - clinit.invokeStaticMethod(C_MAPPED_PROPERTIES)); clinit.invokeStaticMethod(CD_UNKNOWN_PROPERTIES, clinit.readStaticField(C_UNKNOWN)); if (liveReloadPossible) { @@ -468,10 +468,6 @@ public void run() { } // generate sweep for run time configSweepLoop(rtParserBody, readConfig, runTimeConfig, getRegisteredRoots(RUN_TIME), Type.RUNTIME); - - readConfig.invokeStaticMethod(PU_FILTER_UNKNOWN, - readConfig.readStaticField(C_UNKNOWN_RUNTIME), - readConfig.invokeStaticMethod(C_MAPPED_PROPERTIES)); readConfig.invokeStaticMethod(CD_UNKNOWN_PROPERTIES_RT, readConfig.readStaticField(C_UNKNOWN_RUNTIME)); // generate ensure-initialized method @@ -525,33 +521,42 @@ public void run() { private void configSweepLoop(MethodDescriptor parserBody, MethodCreator method, ResultHandle config, Set registeredRoots, Type type) { - ResultHandle nameSet; - ResultHandle iterator; + ResultHandle propertyNames = method.invokeVirtualMethod(SRC_GET_PROPERTY_NAMES, config); + ResultHandle iterator = method.invokeInterfaceMethod(ITRA_ITERATOR, propertyNames); - nameSet = filterProperties(method, config, registeredRoots); - iterator = method.invokeInterfaceMethod(ITRA_ITERATOR, nameSet); + ResultHandle rootSet = method.newInstance(HS_NEW); + for (String registeredRoot : registeredRoots) { + method.invokeVirtualMethod(HS_ADD, rootSet, method.load(registeredRoot)); + } try (BytecodeCreator sweepLoop = method.createScope()) { try (BytecodeCreator hasNext = sweepLoop.ifNonZero(sweepLoop.invokeInterfaceMethod(ITR_HAS_NEXT, iterator)) .trueBranch()) { - ResultHandle key = hasNext.checkCast(hasNext.invokeInterfaceMethod(ITR_NEXT, iterator), String.class); + + // !mappedProperties.contains(new PropertyName(key)) continue sweepLoop; + hasNext.ifNonZero( + hasNext.invokeVirtualMethod(HS_CONTAINS, hasNext.readStaticField(C_MAPPED_PROPERTIES), + hasNext.newInstance(PN_NEW, key))) + .trueBranch().continueScope(sweepLoop); + // NameIterator keyIter = new NameIterator(key); ResultHandle keyIter = hasNext.newInstance(NI_NEW_STRING, key); - BranchResult unknownProperty = hasNext + + // if (PropertiesUtil.isPropertyQuarkusCompoundName(keyIter)) + BranchResult quarkusCompoundName = hasNext .ifNonZero(hasNext.invokeStaticMethod(PU_IS_PROPERTY_QUARKUS_COMPOUND_NAME, keyIter)); - try (BytecodeCreator trueBranch = unknownProperty.trueBranch()) { - ResultHandle unknown; - if (type == Type.BUILD_TIME) { - unknown = trueBranch.readStaticField(C_UNKNOWN); - } else { - unknown = trueBranch.readStaticField(C_UNKNOWN_RUNTIME); - } + try (BytecodeCreator trueBranch = quarkusCompoundName.trueBranch()) { + ResultHandle unknown = type == Type.BUILD_TIME ? trueBranch.readStaticField(C_UNKNOWN) + : trueBranch.readStaticField(C_UNKNOWN_RUNTIME); trueBranch.invokeVirtualMethod(HS_ADD, unknown, key); } + + hasNext.ifNonZero(hasNext.invokeStaticMethod(PU_IS_PROPERTY_IN_ROOTS, key, rootSet)).falseBranch() + .continueScope(sweepLoop); + // if (! keyIter.hasNext()) continue sweepLoop; hasNext.ifNonZero(hasNext.invokeVirtualMethod(NI_HAS_NEXT, keyIter)).falseBranch().continueScope(sweepLoop); - // if (! keyIter.nextSegmentEquals("quarkus")) continue sweepLoop; // parse(config, keyIter); hasNext.invokeStaticMethod(parserBody, config, keyIter); // continue sweepLoop; @@ -560,21 +565,6 @@ private void configSweepLoop(MethodDescriptor parserBody, MethodCreator method, } } - private ResultHandle filterProperties(MethodCreator method, ResultHandle config, Set registeredRoots) { - // Roots - ResultHandle rootSet; - rootSet = method.newInstance(HS_NEW); - for (String registeredRoot : registeredRoots) { - method.invokeVirtualMethod(HS_ADD, rootSet, method.load(registeredRoot)); - } - - // PropertyNames - ResultHandle properties = method.invokeVirtualMethod(SRC_GET_PROPERTY_NAMES, config); - - // Filtered Properties - return method.invokeStaticMethod(PU_FILTER_PROPERTIES_IN_ROOTS, properties, rootSet); - } - private Set getRegisteredRoots(ConfigPhase configPhase) { Set registeredRoots = new HashSet<>(); for (RootDefinition root : roots) { @@ -1190,13 +1180,7 @@ private FieldDescriptor getOrCreateConverterInstance(Field field, ConverterType return fd; } - static final MethodDescriptor KM_NEW = MethodDescriptor.ofConstructor(KeyMap.class); - static final MethodDescriptor KM_FIND_OR_ADD = MethodDescriptor.ofMethod(KeyMap.class, "findOrAdd", KeyMap.class, - String.class); - static final MethodDescriptor KM_PUT_ROOT_VALUE = MethodDescriptor.ofMethod(KeyMap.class, "putRootValue", Object.class, - Object.class); - - private void generateUnknownFilter() { + private void generateMappedProperties() { Set names = new HashSet<>(); for (ConfigClass buildTimeMapping : buildTimeConfigResult.getBuildTimeMappings()) { names.addAll(ConfigMappings.getProperties(buildTimeMapping).keySet()); @@ -1207,17 +1191,15 @@ private void generateUnknownFilter() { for (ConfigClass runtimeConfigMapping : buildTimeConfigResult.getRunTimeMappings()) { names.addAll(ConfigMappings.getProperties(runtimeConfigMapping).keySet()); } + Set propertyNames = PropertiesUtil.toPropertyNames(names); - // Add a method that generates a KeyMap that can check if a property is mapped by a @ConfigMapping - MethodCreator mc = cc.getMethodCreator(C_MAPPED_PROPERTIES); + MethodCreator mc = cc.getMethodCreator(C_GENERATE_MAPPED_PROPERTIES); mc.setModifiers(Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC); - ResultHandle keyMap = mc.newInstance(KM_NEW); - for (String name : names) { - mc.invokeVirtualMethod(KM_PUT_ROOT_VALUE, mc.invokeVirtualMethod(KM_FIND_OR_ADD, keyMap, mc.load(name)), - mc.load(true)); + ResultHandle set = mc.newInstance(HS_NEW); + for (PropertyName propertyName : propertyNames) { + mc.invokeVirtualMethod(HS_ADD, set, mc.newInstance(PN_NEW, mc.load(propertyName.getName()))); } - - mc.returnValue(keyMap); + mc.returnValue(set); mc.close(); } diff --git a/core/runtime/src/main/java/io/quarkus/runtime/configuration/PropertiesUtil.java b/core/runtime/src/main/java/io/quarkus/runtime/configuration/PropertiesUtil.java index 0a7b27e0f52cf..18978d1714028 100644 --- a/core/runtime/src/main/java/io/quarkus/runtime/configuration/PropertiesUtil.java +++ b/core/runtime/src/main/java/io/quarkus/runtime/configuration/PropertiesUtil.java @@ -1,86 +1,14 @@ package io.quarkus.runtime.configuration; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; +import java.util.HashMap; +import java.util.Map; import java.util.Set; -import io.smallrye.config.KeyMap; +import io.smallrye.config.PropertyName; public class PropertiesUtil { private PropertiesUtil() { - } - - /** - * @deprecated Use {@link PropertiesUtil#filterPropertiesInRoots(Iterable, Set)} instead. - */ - @Deprecated(forRemoval = true) - public static boolean isPropertyInRoot(Set roots, NameIterator propertyName) { - for (String root : roots) { - // match everything - if (root.length() == 0) { - return true; - } - - // A sub property from a namespace is always bigger - if (propertyName.getName().length() <= root.length()) { - continue; - } - - final NameIterator rootNi = new NameIterator(root); - // compare segments - while (rootNi.hasNext()) { - String segment = rootNi.getNextSegment(); - if (!propertyName.hasNext()) { - propertyName.goToStart(); - break; - } - - final String nextSegment = propertyName.getNextSegment(); - if (!segment.equals(nextSegment)) { - propertyName.goToStart(); - break; - } - - rootNi.next(); - propertyName.next(); - - // root has no more segments, and we reached this far so everything matched. - // on top, property still has more segments to do the mapping. - if (!rootNi.hasNext() && propertyName.hasNext()) { - propertyName.goToStart(); - return true; - } - } - } - - return false; - } - - public static Iterable filterPropertiesInRoots(final Iterable properties, final Set roots) { - if (roots.isEmpty()) { - return properties; - } - - // Will match everything, so no point in filtering - if (roots.contains("")) { - return properties; - } - - List matchedProperties = new ArrayList<>(); - for (String property : properties) { - // This is a Quarkus compound name, usually by setting something like `quarkus.foo.bar` in the YAML source - // TODO - We let it through to match it later again to place it in the right unknown reporting (static or runtime). We can improve this too. - if (property.startsWith("\"quarkus.")) { - matchedProperties.add(property); - continue; - } - - if (isPropertyInRoots(property, roots)) { - matchedProperties.add(property); - } - } - return matchedProperties; + throw new IllegalStateException("Utility class"); } public static boolean isPropertyInRoots(final String property, final Set roots) { @@ -121,23 +49,27 @@ public static boolean isPropertyQuarkusCompoundName(NameIterator propertyName) { return propertyName.getName().startsWith("\"quarkus."); } - /** - * Removes false positives of configuration properties marked as unknown. To populate the old @ConfigRoot, all - * properties are iterated and matched against known roots. With @ConfigMapping the process is different, so - * properties that are known to @ConfigMapping are not known to the @ConfigRoot, so they will be marked as being - * unknown. It is a bit easier to just double-check on the unknown properties and remove these false positives by - * matching them against the known properties of @ConfigMapping. - * - * @param unknownProperties the collected unknown properties from the old @ConfigRoot mapping - * @param filterPatterns the mapped patterns from the discovered @ConfigMapping - */ - public static void filterUnknown(Set unknownProperties, KeyMap filterPatterns) { - Set toRemove = new HashSet<>(); - for (String unknownProperty : unknownProperties) { - if (filterPatterns.hasRootValue(unknownProperty)) { - toRemove.add(unknownProperty); + public static Set toPropertyNames(final Set names) { + Map propertyNames = new HashMap<>(); + for (String name : names) { + PropertyName propertyName = new PropertyName(name); + if (propertyNames.containsKey(propertyName)) { + String existing = propertyNames.remove(propertyName); + if (existing.length() < name.length()) { + propertyNames.put(new PropertyName(existing), existing); + } else if (existing.length() > name.length()) { + propertyNames.put(propertyName, name); + } else { + if (existing.indexOf('*') <= name.indexOf('*')) { + propertyNames.put(new PropertyName(existing), existing); + } else { + propertyNames.put(propertyName, name); + } + } + } else { + propertyNames.put(propertyName, name); } } - unknownProperties.removeAll(toRemove); + return propertyNames.keySet(); } }