HHH-15100 Limitation of metamodel imports cache causes severe perform…

…ance drops in large projects

hibernate-core/src/main/java/org/hibernate/metamodel/internal/MetamodelImpl.java

@@ -91,7 +91,8 @@ public class MetamodelImpl implements MetamodelImplementor, Serializable {
 
 	private final SessionFactoryImplementor sessionFactory;
 
-	private final Map<String,String> imports = new ConcurrentHashMap<>();
+	private final Map<String,String> knownValidImports = new ConcurrentHashMap<>();
+	private final Map<String,String> knownInvalidImports = new ConcurrentHashMap<>();
 	private final Map<String,EntityPersister> entityPersisterMap = new ConcurrentHashMap<>();
 	private final Map<Class,String> entityProxyInterfaceMap = new ConcurrentHashMap<>();
 	private final Map<String,CollectionPersister> collectionPersisterMap = new ConcurrentHashMap<>();
@@ -155,7 +156,7 @@ public MetamodelImpl(SessionFactoryImplementor sessionFactory, TypeConfiguration
 	 * @param jpaMetaModelPopulationSetting Should the JPA Metamodel be built as well?
 	 */
 	public void initialize(MetadataImplementor mappingMetadata, JpaMetaModelPopulationSetting jpaMetaModelPopulationSetting) {
-		this.imports.putAll( mappingMetadata.getImports() );
+		this.knownValidImports.putAll( mappingMetadata.getImports() );
 
 		primeSecondLevelCacheRegions( mappingMetadata );
 
@@ -623,33 +624,37 @@ public <X> EntityTypeDescriptor<X> entity(String entityName) {
 	}
 
 	@Override
-	public String getImportedClassName(String className) {
-		String result = imports.get( className );
-		if ( result == null ) {
-			try {
-				sessionFactory.getServiceRegistry().getService( ClassLoaderService.class ).classForName( className );
-				imports.put( className, className );
-				return className;
-			}
-			catch ( ClassLoadingException cnfe ) {
-				// This check doesn't necessarily mean that the map can't exceed 1000 elements because
-				// new entries might be added _while_ performing the check (making it 1000+ since size() isn't
-				// synchronized). Regardless, this would pass as "good enough" to prevent the map from growing
-				// above a certain threshold, thus, avoiding memory issues.
-				if ( imports.size() < 1_000 ) {
-					imports.put( className, INVALID_IMPORT );
-				}
-				return null;
-			}
+	public String getImportedClassName(final String className) {
+		final String result = knownValidImports.get( className );
+		if ( result != null ) {
+			//optimal path:
+			return result;
 		}
 		else {
-			// explicitly check for same instance
-			//noinspection StringEquality
-			if ( result == INVALID_IMPORT ) {
+			//check the negative cache first, to avoid ClassLoadingException for commonly used strings which aren't class names:
+			if ( knownInvalidImports.containsKey( className ) ) {
 				return null;
 			}
 			else {
-				return result;
+				//either we've not seen this string yet, or the negative cache has grown too much;
+				//either way we need to attempt a regular class load:
+				try {
+					sessionFactory.getServiceRegistry().getService( ClassLoaderService.class ).classForName( className );
+					//Store this information in the cache:
+					knownValidImports.put( className, className );
+					return className;
+				}
+				catch ( ClassLoadingException cnfe ) {
+					// This check doesn't necessarily mean that the map can't exceed 1000 elements because
+					// new entries might be added _while_ performing the check (making it 1000+ since size() isn't
+					// synchronized). Regardless, this would pass as "good enough" to prevent the map from growing
+					// above a certain threshold, thus, avoiding memory issues.
+					if ( knownInvalidImports.size() < 1_000 ) {
+						//Store this in the negative cache, but only if it's not getting too large.
+						knownInvalidImports.put( className, INVALID_IMPORT );
+					}
+					return null;
+				}
 			}
 		}
 	}

hibernate-core/src/test/java/org/hibernate/test/hql/QuerySplitterTest.java

@@ -61,17 +61,22 @@ public void testMemoryConsumptionOfFailedImportsCache() throws NoSuchFieldExcept
 						sessionFactory()
 				) );
 
-		MetamodelImpl metamodel = (MetamodelImpl) sessionFactory().getMetamodel();
+		Map<String, String> validImports = extractMapFromMetamodel("knownValidImports");
+		Map<String, String> invalidImports  = extractMapFromMetamodel("knownInvalidImports");
 
-		Field field = MetamodelImpl.class.getDeclaredField( "imports" );
-		field.setAccessible( true );
-
-		//noinspection unchecked
-		Map<String, String> imports = (Map<String, String>) field.get( metamodel );
+		assertEquals( 2, validImports.size() );
 
 		// VERY hard-coded, but considering the possibility of a regression of a memory-related issue,
 		// it should be worth it
-		assertEquals( 1000, imports.size() );
+		assertEquals( 1_000, invalidImports.size() );
+	}
+
+	private Map<String, String> extractMapFromMetamodel(String fieldName) throws NoSuchFieldException, IllegalAccessException {
+		MetamodelImpl metamodel = (MetamodelImpl) sessionFactory().getMetamodel();
+		Field field = MetamodelImpl.class.getDeclaredField( fieldName );
+		field.setAccessible( true );
+		//noinspection unchecked
+		return (Map<String, String>) field.get( metamodel );
 	}
 
 	@Test