Skip to content

Commit 4861d55

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 3c39da9 commit 4861d55

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
@@ -130,6 +130,13 @@ private EntityInstantiator createEntityInstantiator(PersistentEntity<?, ?> entit
130130
try {
131131
return doCreateEntityInstantiator(entity);
132132
} catch (Throwable ex) {
133+
134+
if (LOGGER.isDebugEnabled()) {
135+
LOGGER.debug(
136+
String.format("Cannot create entity instantiator for %s. Falling back to ReflectionEntityInstantiator.",
137+
entity.getName()),
138+
ex);
139+
}
133140
return ReflectionEntityInstantiator.INSTANCE;
134141
}
135142
}
@@ -376,12 +383,22 @@ public Class<?> generateCustomInstantiatorClass(PersistentEntity<?, ?> entity,
376383
@Nullable PreferredConstructor<?, ?> constructor) {
377384

378385
String className = generateClassName(entity);
379-
byte[] bytecode = generateBytecode(className, entity, constructor);
380-
381386
Class<?> type = entity.getType();
387+
ClassLoader classLoader = type.getClassLoader();
388+
389+
if (ClassUtils.isPresent(className, classLoader)) {
390+
391+
try {
392+
return ClassUtils.forName(className, classLoader);
393+
} catch (Exception o_O) {
394+
throw new IllegalStateException(o_O);
395+
}
396+
}
397+
398+
byte[] bytecode = generateBytecode(className, entity, constructor);
382399

383400
try {
384-
return ReflectUtils.defineClass(className, bytecode, type.getClassLoader(), type.getProtectionDomain(), type);
401+
return ReflectUtils.defineClass(className, bytecode, classLoader, type.getProtectionDomain(), type);
385402
} catch (Exception e) {
386403
throw new IllegalStateException(e);
387404
}

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;
@@ -40,6 +41,8 @@
4041
import org.springframework.data.mapping.model.ClassGeneratingEntityInstantiator.ObjectInstantiator;
4142
import org.springframework.data.mapping.model.ClassGeneratingEntityInstantiatorUnitTests.Outer.Inner;
4243
import org.springframework.data.util.ClassTypeInformation;
44+
import org.springframework.data.util.TypeInformation;
45+
import org.springframework.test.util.ReflectionTestUtils;
4346
import org.springframework.util.ReflectionUtils;
4447

4548
/**
@@ -211,6 +214,9 @@ void instantiateObjCtorNoArgs() {
211214
assertThat(instance).isInstanceOf(ObjCtorNoArgs.class);
212215
assertThat(((ObjCtorNoArgs) instance).ctorInvoked).isTrue();
213216
});
217+
218+
ClassGeneratingEntityInstantiator classGeneratingEntityInstantiator = new ClassGeneratingEntityInstantiator();
219+
classGeneratingEntityInstantiator.createInstance(entity, provider);
214220
}
215221

216222
@Test // DATACMNS-578, DATACMNS-1126
@@ -361,6 +367,26 @@ void shouldNotInstantiateClassWithPrivateConstructor() {
361367
assertThat(this.instance.shouldUseReflectionEntityInstantiator(entity)).isTrue();
362368
}
363369

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

0 commit comments

Comments
 (0)