Skip to content

Commit bd280f8

Browse files
committed
Reuse generated entity instantiators from ClassGeneratingEntityInstantiator instances.
We now reuse generated ObjectInstantiator classes instead of attempting to redefine classes with the same name. Redefinition leads to LinkageError and thus to the use of reflection-based instantiators and an increased allocation rate due to the failed attempt of class redeclaration. Closes: #2446.
1 parent 345e18c commit bd280f8

File tree

2 files changed

+46
-3
lines changed

2 files changed

+46
-3
lines changed

src/main/java/org/springframework/data/mapping/model/ClassGeneratingEntityInstantiator.java

+20-3
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,13 @@ private EntityInstantiator createEntityInstantiator(PersistentEntity<?, ?> entit
124124
try {
125125
return doCreateEntityInstantiator(entity);
126126
} catch (Throwable ex) {
127+
128+
if (LOGGER.isDebugEnabled()) {
129+
LOGGER.debug(
130+
String.format("Cannot create entity instantiator for %s. Falling back to ReflectionEntityInstantiator.",
131+
entity.getName()),
132+
ex);
133+
}
127134
return ReflectionEntityInstantiator.INSTANCE;
128135
}
129136
}
@@ -336,12 +343,22 @@ public Class<?> generateCustomInstantiatorClass(PersistentEntity<?, ?> entity,
336343
@Nullable PreferredConstructor<?, ?> constructor) {
337344

338345
String className = generateClassName(entity);
339-
byte[] bytecode = generateBytecode(className, entity, constructor);
340-
341346
Class<?> type = entity.getType();
347+
ClassLoader classLoader = type.getClassLoader();
348+
349+
if (ClassUtils.isPresent(className, classLoader)) {
350+
351+
try {
352+
return ClassUtils.forName(className, classLoader);
353+
} catch (Exception o_O) {
354+
throw new IllegalStateException(o_O);
355+
}
356+
}
357+
358+
byte[] bytecode = generateBytecode(className, entity, constructor);
342359

343360
try {
344-
return ReflectUtils.defineClass(className, bytecode, type.getClassLoader(), type.getProtectionDomain(), type);
361+
return ReflectUtils.defineClass(className, bytecode, classLoader, type.getProtectionDomain(), type);
345362
} catch (Exception e) {
346363
throw new IllegalStateException(e);
347364
}

src/test/java/org/springframework/data/mapping/model/ClassGeneratingEntityInstantiatorUnitTests.java

+26
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.lang.reflect.Constructor;
2424
import java.util.Arrays;
2525
import java.util.List;
26+
import java.util.Map;
2627
import java.util.stream.IntStream;
2728

2829
import org.junit.jupiter.api.Test;
@@ -39,6 +40,8 @@
3940
import org.springframework.data.mapping.PreferredConstructor.Parameter;
4041
import org.springframework.data.mapping.model.ClassGeneratingEntityInstantiator.ObjectInstantiator;
4142
import org.springframework.data.mapping.model.ClassGeneratingEntityInstantiatorUnitTests.Outer.Inner;
43+
import org.springframework.data.util.TypeInformation;
44+
import org.springframework.test.util.ReflectionTestUtils;
4245
import org.springframework.util.ReflectionUtils;
4346

4447
/**
@@ -210,6 +213,9 @@ void instantiateObjCtorNoArgs() {
210213
assertThat(instance).isInstanceOf(ObjCtorNoArgs.class);
211214
assertThat(((ObjCtorNoArgs) instance).ctorInvoked).isTrue();
212215
});
216+
217+
ClassGeneratingEntityInstantiator classGeneratingEntityInstantiator = new ClassGeneratingEntityInstantiator();
218+
classGeneratingEntityInstantiator.createInstance(entity, provider);
213219
}
214220

215221
@Test // DATACMNS-578, DATACMNS-1126
@@ -360,6 +366,26 @@ void shouldNotInstantiateClassWithPrivateConstructor() {
360366
assertThat(this.instance.shouldUseReflectionEntityInstantiator(entity)).isTrue();
361367
}
362368

369+
@Test // GH-2446
370+
void shouldReuseGeneratedClasses() {
371+
372+
prepareMocks(ProtectedInnerClass.class);
373+
374+
this.instance.createInstance(entity, provider);
375+
376+
ClassGeneratingEntityInstantiator instantiator = new ClassGeneratingEntityInstantiator();
377+
instantiator.createInstance(entity, provider);
378+
379+
Map<TypeInformation<?>, EntityInstantiator> first = (Map<TypeInformation<?>, EntityInstantiator>) ReflectionTestUtils
380+
.getField(this.instance, "entityInstantiators");
381+
382+
Map<TypeInformation<?>, EntityInstantiator> second = (Map<TypeInformation<?>, EntityInstantiator>) ReflectionTestUtils
383+
.getField(instantiator, "entityInstantiators");
384+
385+
assertThat(first.get(null)).isNotNull().isNotInstanceOf(Enum.class);
386+
assertThat(second.get(null)).isNotNull().isNotInstanceOf(Enum.class);
387+
}
388+
363389
@Test // DATACMNS-1422
364390
void shouldUseReflectionIfFrameworkTypesNotVisible() throws Exception {
365391

0 commit comments

Comments
 (0)