Skip to content

Commit 95d5954

Browse files
committed
Improve initialization of constant pool scanning configuration
In BytecodeTransformerBuildItem, we can define a set of classes which will be used to determine if a class is affected by a transformation by by checking the constant pool. This is most notably (and I think exclusively) used as an optimization for the Panache transformation of public fields -> accessors. When you have an application with a lot of entities, all the BytecodeTransformerBuildItem targeting the classes using these entities are carrying the Set of all the entities. And we used copy the Set over and over for each class affected by the transformation, which was generating a lot of useless allocations. Obviously, this optimization only works if we don't use the constant pool scanning optimization for other transformations. And that why, I ended up adding another optimization: we now only pass to the transformer the entities we have detected as being used in this class instead of the whole list of entities. While doing that should make the first optimization a lot less useful, I think it's still good to keep it around.
1 parent 960fe2b commit 95d5954

File tree

2 files changed

+60
-28
lines changed

2 files changed

+60
-28
lines changed

core/deployment/src/main/java/io/quarkus/deployment/steps/ClassTransformingBuildStep.java

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import java.util.HashSet;
1717
import java.util.List;
1818
import java.util.Map;
19+
import java.util.Map.Entry;
1920
import java.util.Objects;
2021
import java.util.Set;
2122
import java.util.concurrent.Callable;
@@ -101,19 +102,16 @@ TransformedClassesBuildItem handleClassTransformation(List<BytecodeTransformerBu
101102
for (BytecodeTransformerBuildItem i : bytecodeTransformerBuildItems) {
102103
bytecodeTransformers.computeIfAbsent(i.getClassToTransform(), (h) -> new ArrayList<>())
103104
.add(i);
104-
if (i.getRequireConstPoolEntry() == null || i.getRequireConstPoolEntry().isEmpty()) {
105-
noConstScanning.add(i.getClassToTransform());
106-
} else {
107-
constScanning.computeIfAbsent(i.getClassToTransform(), (s) -> new HashSet<>())
108-
.addAll(i.getRequireConstPoolEntry());
109-
}
110105
if (!i.isCacheable()) {
111106
nonCacheable.add(i.getClassToTransform());
112107
}
113108
classReaderOptions.merge(i.getClassToTransform(), i.getClassReaderOptions(),
114109
// class reader options are bit flags (see org.objectweb.asm.ClassReader)
115110
(oldValue, newValue) -> oldValue | newValue);
116111
}
112+
113+
initializeConstPoolScanningConfiguration(bytecodeTransformers, noConstScanning, constScanning);
114+
117115
QuarkusClassLoader cl = (QuarkusClassLoader) Thread.currentThread().getContextClassLoader();
118116
Map<String, Path> transformedToArchive = new ConcurrentHashMap<>();
119117
// now copy all the contents to the runner jar
@@ -288,6 +286,39 @@ public TransformedClassesBuildItem.TransformedClass call() throws Exception {
288286
return new TransformedClassesBuildItem(transformedClassesByJar);
289287
}
290288

289+
private static void initializeConstPoolScanningConfiguration(
290+
Map<String, List<BytecodeTransformerBuildItem>> bytecodeTransformers, Set<String> noConstScanning,
291+
Map<String, Set<String>> constScanning) {
292+
// this option is not widely used and it is mostly used by the Panache transformer AFAIK
293+
// in this case, we have all the Panache entities in the list, which might be quite a lot when you have a lot of entities
294+
// and it's worth avoiding copying the entries over and over as we used to create a new copy of the set for each entity
295+
for (Entry<String, List<BytecodeTransformerBuildItem>> bytecodeTransformersPerClassEntry : bytecodeTransformers
296+
.entrySet()) {
297+
List<BytecodeTransformerBuildItem> transformersRequiringConstPoolEntries = new ArrayList<>(
298+
bytecodeTransformersPerClassEntry.getValue().size());
299+
int requiredConstPoolEntriesNumber = 0;
300+
for (BytecodeTransformerBuildItem transformer : bytecodeTransformersPerClassEntry.getValue()) {
301+
if (transformer.getRequireConstPoolEntry() != null && !transformer.getRequireConstPoolEntry().isEmpty()) {
302+
transformersRequiringConstPoolEntries.add(transformer);
303+
requiredConstPoolEntriesNumber += transformer.getRequireConstPoolEntry().size();
304+
}
305+
}
306+
if (transformersRequiringConstPoolEntries.isEmpty()) {
307+
noConstScanning.add(bytecodeTransformersPerClassEntry.getKey());
308+
} else if (transformersRequiringConstPoolEntries.size() == 1) {
309+
constScanning.put(bytecodeTransformersPerClassEntry.getKey(),
310+
transformersRequiringConstPoolEntries.get(0).getRequireConstPoolEntry());
311+
} else {
312+
// we don't do that often, but let's avoid having to resize the underlying HashMap as it can be quite big
313+
Set<String> requiredConstPoolEntries = new HashSet<>((int) Math.ceil(requiredConstPoolEntriesNumber / 0.75f));
314+
for (BytecodeTransformerBuildItem transformer : transformersRequiringConstPoolEntries) {
315+
requiredConstPoolEntries.addAll(transformer.getRequireConstPoolEntry());
316+
}
317+
constScanning.put(bytecodeTransformersPerClassEntry.getKey(), requiredConstPoolEntries);
318+
}
319+
}
320+
}
321+
291322
private void copyTransformedClasses(Path originalClassesPath,
292323
Set<TransformedClassesBuildItem.TransformedClass> transformedClasses) {
293324
if ((transformedClasses == null) || transformedClasses.isEmpty()) {

extensions/panache/panache-hibernate-common/deployment/src/main/java/io/quarkus/panache/hibernate/common/deployment/PanacheHibernateCommonResourceProcessor.java

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
package io.quarkus.panache.hibernate.common.deployment;
22

33
import java.lang.reflect.Modifier;
4+
import java.util.HashMap;
45
import java.util.HashSet;
6+
import java.util.Map;
7+
import java.util.Map.Entry;
58
import java.util.Optional;
69
import java.util.Set;
710

@@ -120,34 +123,32 @@ void replaceFieldAccesses(CombinedIndexBuildItem index,
120123
}
121124

122125
// Replace field access in application code with calls to accessors
123-
Set<String> entityClassNamesInternal = new HashSet<>();
124-
for (String entityClassName : entitiesWithExternallyAccessibleFields) {
125-
entityClassNamesInternal.add(entityClassName.replace(".", "/"));
126-
}
127-
128126
PanacheFieldAccessEnhancer panacheFieldAccessEnhancer = new PanacheFieldAccessEnhancer(modelInfo);
129-
Set<String> produced = new HashSet<>();
127+
128+
Map<String, Set<String>> classesUsingEntities = new HashMap<>();
130129
// transform all users of those classes
131130
for (String entityClassName : entitiesWithExternallyAccessibleFields) {
132131
for (ClassInfo userClass : index.getIndex().getKnownUsers(entityClassName)) {
133-
String cn = userClass.name().toString('.');
134-
if (produced.contains(cn)) {
135-
continue;
136-
}
137-
produced.add(cn);
138-
//The following build item is not marked as CacheAble intentionally: see also https://github.com/quarkusio/quarkus/pull/40192#discussion_r1590605375.
139-
//It shouldn't be too hard to improve on this by checking the related entities haven't been changed
140-
//via LiveReloadBuildItem (#isLiveReload() && #getChangeInformation()) but I'm not comfortable in making this
141-
//change without having solid integration tests.
142-
final BytecodeTransformerBuildItem transformation = new BytecodeTransformerBuildItem.Builder()
143-
.setClassToTransform(cn)
144-
.setCacheable(false)//TODO this would be nice to improve on: see note above.
145-
.setVisitorFunction(panacheFieldAccessEnhancer)
146-
.setRequireConstPoolEntry(entityClassNamesInternal)
147-
.build();
148-
transformers.produce(transformation);
132+
String userClassName = userClass.name().toString('.');
133+
134+
classesUsingEntities.computeIfAbsent(userClassName, k -> new HashSet<>())
135+
.add(entityClassName.replace(".", "/"));
149136
}
150137
}
138+
139+
for (Entry<String, Set<String>> classUsingEntities : classesUsingEntities.entrySet()) {
140+
//The following build item is not marked as CacheAble intentionally: see also https://github.com/quarkusio/quarkus/pull/40192#discussion_r1590605375.
141+
//It shouldn't be too hard to improve on this by checking the related entities haven't been changed
142+
//via LiveReloadBuildItem (#isLiveReload() && #getChangeInformation()) but I'm not comfortable in making this
143+
//change without having solid integration tests.
144+
final BytecodeTransformerBuildItem transformation = new BytecodeTransformerBuildItem.Builder()
145+
.setClassToTransform(classUsingEntities.getKey())
146+
.setCacheable(false) // TODO this would be nice to improve on: see note above.
147+
.setVisitorFunction(panacheFieldAccessEnhancer)
148+
.setRequireConstPoolEntry(classUsingEntities.getValue())
149+
.build();
150+
transformers.produce(transformation);
151+
}
151152
}
152153

153154
private EntityModel createEntityModel(ClassInfo classInfo) {

0 commit comments

Comments
 (0)