From 1945180569e7f86f8b5633418c5a1af42db3c8e2 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Thu, 12 Jul 2018 19:03:04 +0200 Subject: [PATCH] HHH-12791 Cache the Component type to avoid generating one proxy per call While Javassist only generates one proxy as the name is stable, ByteBuddy uses random names and thus generates a new proxy for every call, leading to the generation of 18 different proxies for the other test of the test class. We can't do better than using a volatile/synchronized pattern as the Component is not fully initialized in the constructor. Maybe we could take the risk of admitting that the getType() method is called at least once before we pass the element to a multi-threaded environment but that's a bet I don't want to take alone. --- .../java/org/hibernate/mapping/Component.java | 38 ++++++++++++++----- .../proxy/ComponentBasicProxyTest.java | 33 ++++++++++++++++ 2 files changed, 61 insertions(+), 10 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/Component.java b/hibernate-core/src/main/java/org/hibernate/mapping/Component.java index 53a54777d5..f988a8ba4e 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/Component.java +++ b/hibernate-core/src/main/java/org/hibernate/mapping/Component.java @@ -52,6 +52,9 @@ public class Component extends SimpleValue implements MetaAttributable { private java.util.Map tuplizerImpls; + // cache the status of the type + private volatile Type type; + /** * @deprecated User {@link Component#Component(MetadataBuildingContext, PersistentClass)} instead. */ @@ -209,13 +212,28 @@ public class Component extends SimpleValue implements MetaAttributable { @Override public Type getType() throws MappingException { - // TODO : temporary initial step towards HHH-1907 - final ComponentMetamodel metamodel = new ComponentMetamodel( - this, - getMetadata().getMetadataBuildingOptions() - ); - final TypeFactory factory = getMetadata().getTypeConfiguration().getTypeResolver().getTypeFactory(); - return isEmbedded() ? factory.embeddedComponent( metamodel ) : factory.component( metamodel ); + // Resolve the type of the value once and for all as this operation generates a proxy class + // for each invocation. + // Unfortunately, there's no better way of doing that as none of the classes are immutable and + // we can't know for sure the current state of the property or the value. + Type localType = type; + + if ( localType == null ) { + synchronized ( this ) { + if ( type == null ) { + // TODO : temporary initial step towards HHH-1907 + final ComponentMetamodel metamodel = new ComponentMetamodel( + this, + getMetadata().getMetadataBuildingOptions() + ); + final TypeFactory factory = getMetadata().getTypeConfiguration().getTypeResolver().getTypeFactory(); + localType = isEmbedded() ? factory.embeddedComponent( metamodel ) : factory.component( metamodel ); + type = localType; + } + } + } + + return localType; } @Override @@ -288,15 +306,15 @@ public class Component extends SimpleValue implements MetaAttributable { } return result; } - + public boolean isKey() { return isKey; } - + public void setKey(boolean isKey) { this.isKey = isKey; } - + public boolean hasPojoRepresentation() { return componentClassName!=null; } diff --git a/hibernate-core/src/test/java/org/hibernate/test/component/proxy/ComponentBasicProxyTest.java b/hibernate-core/src/test/java/org/hibernate/test/component/proxy/ComponentBasicProxyTest.java index 59debe8ff6..c41a87e254 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/component/proxy/ComponentBasicProxyTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/component/proxy/ComponentBasicProxyTest.java @@ -7,11 +7,20 @@ package org.hibernate.test.component.proxy; import static org.hibernate.testing.transaction.TransactionUtil.doInJPA; +import static org.junit.Assert.assertEquals; import java.util.List; +import org.hibernate.EntityMode; +import org.hibernate.boot.Metadata; +import org.hibernate.boot.MetadataSources; +import org.hibernate.boot.registry.StandardServiceRegistry; +import org.hibernate.boot.registry.StandardServiceRegistryBuilder; import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase; +import org.hibernate.mapping.PersistentClass; +import org.hibernate.test.annotations.basic.CollectionAsBasicTest.DelimitedStringsType; import org.hibernate.testing.TestForIssue; +import org.hibernate.type.ComponentType; import org.junit.Test; /** @@ -44,4 +53,28 @@ public class ComponentBasicProxyTest extends BaseEntityManagerFunctionalTestCase entityManager.remove( adult ); } ); } + + @Test + @TestForIssue(jiraKey = "HHH-12791") + public void testOnlyOneProxyClassGenerated() { + StandardServiceRegistry ssr = new StandardServiceRegistryBuilder().build(); + + try { + Metadata metadata = new MetadataSources( ssr ).addAnnotatedClass( Person.class ) + .getMetadataBuilder().applyBasicType( new DelimitedStringsType() ) + .build(); + PersistentClass persistentClass = metadata.getEntityBinding( Person.class.getName() ); + + ComponentType componentType1 = (ComponentType) persistentClass.getIdentifierMapper().getType(); + Object instance1 = componentType1.instantiate( EntityMode.POJO ); + + ComponentType componentType2 = (ComponentType) persistentClass.getIdentifierMapper().getType(); + Object instance2 = componentType2.instantiate( EntityMode.POJO ); + + assertEquals( instance1.getClass(), instance2.getClass() ); + } + finally { + StandardServiceRegistryBuilder.destroy( ssr ); + } + } }