diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/SqmTreeTransformationLogger.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/SqmTreeTransformationLogger.java index 9034d26a1309..5230d3f5dc9e 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/SqmTreeTransformationLogger.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/SqmTreeTransformationLogger.java @@ -16,6 +16,4 @@ public interface SqmTreeTransformationLogger { Logger LOGGER = Logger.getLogger( LOGGER_NAME ); - boolean TRACE_ENABLED = LOGGER.isTraceEnabled(); - boolean DEBUG_ENABLED = LOGGER.isDebugEnabled(); } diff --git a/hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/ResourceRegistryStandardImpl.java b/hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/ResourceRegistryStandardImpl.java index ee6070ce55c7..2baeb3d30fc4 100644 --- a/hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/ResourceRegistryStandardImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/ResourceRegistryStandardImpl.java @@ -38,6 +38,7 @@ public final class ResourceRegistryStandardImpl implements ResourceRegistry { private static final CoreMessageLogger log = CoreLogging.messageLogger( ResourceRegistryStandardImpl.class ); + private static final boolean IS_TRACE_ENABLED = log.isTraceEnabled(); // Dummy value to associate with an Object in the backing Map when we use it as a set: private static final Object PRESENT = new Object(); @@ -50,12 +51,8 @@ public final class ResourceRegistryStandardImpl implements ResourceRegistry { private final JdbcEventHandler jdbcEventHandler; private final HashMap> xref = new HashMap<>(); - private HashMap unassociatedResultSets; - - private ArrayList blobs; - private ArrayList clobs; - private ArrayList nclobs; + private ExtendedState ext; private Statement lastQuery; public ResourceRegistryStandardImpl() { @@ -69,15 +66,12 @@ public ResourceRegistryStandardImpl(JdbcEventHandler jdbcEventHandler) { @Override public boolean hasRegisteredResources() { return hasRegistered( xref ) - || hasRegistered( unassociatedResultSets ) - || hasRegistered( blobs ) - || hasRegistered( clobs ) - || hasRegistered( nclobs ); + || ext != null && ext.hasRegisteredResources(); } @Override public void register(Statement statement, boolean cancelable) { - log.tracef( "Registering statement [%s]", statement ); + if ( IS_TRACE_ENABLED ) log.tracef( "Registering statement [%s]", statement ); HashMap previousValue = xref.putIfAbsent( statement, EMPTY ); if ( previousValue != null ) { @@ -91,7 +85,7 @@ public void register(Statement statement, boolean cancelable) { @Override public void release(Statement statement) { - log.tracev( "Releasing statement [{0}]", statement ); + if ( IS_TRACE_ENABLED ) log.tracev( "Releasing statement [{0}]", statement ); final HashMap resultSets = xref.remove( statement ); if ( resultSets != null ) { @@ -112,7 +106,7 @@ public void release(Statement statement) { @Override public void release(ResultSet resultSet, Statement statement) { - log.tracef( "Releasing result set [%s]", resultSet ); + if ( IS_TRACE_ENABLED ) log.tracef( "Releasing result set [%s]", resultSet ); if ( statement == null ) { try { @@ -142,9 +136,8 @@ public void release(ResultSet resultSet, Statement statement) { } } else { - final Object removed = unassociatedResultSets == null ? null : unassociatedResultSets.remove( resultSet ); - if ( removed == null ) { - log.unregisteredResultSetWithoutStatement(); + if ( ext != null ) { + ext.releaseUnassociatedResult( resultSet ); } } close( resultSet ); @@ -164,7 +157,7 @@ private static void releaseXref(final Statement s, final HashMap(); - } - unassociatedResultSets.put( resultSet, PRESENT ); + getExtendedStateForWrite().storeUnassociatedResultset( resultSet ); + } + } + + private ExtendedState getExtendedStateForWrite() { + if ( this.ext == null ) { + this.ext = new ExtendedState(); } + return this.ext; } private JDBCException convert(SQLException e, String s) { @@ -254,56 +251,46 @@ private JDBCException convert(SQLException e, String s) { @Override public void register(Blob blob) { - if ( blobs == null ) { - blobs = new ArrayList<>(); - } - - blobs.add( blob ); + getExtendedStateForWrite().registerBlob( blob ); } @Override - public void release(Blob blob) { - if ( blobs == null ) { + public void release(final Blob blob) { + if ( ext == null || ext.blobs == null ) { log.debug( "Request to release Blob, but appears no Blobs have ever been registered" ); return; } - blobs.remove( blob ); + ext.blobs.remove( blob ); } @Override - public void register(Clob clob) { - if ( clobs == null ) { - clobs = new ArrayList<>(); - } - clobs.add( clob ); + public void register(final Clob clob) { + getExtendedStateForWrite().registerClob( clob ); } @Override - public void release(Clob clob) { - if ( clobs == null ) { + public void release(final Clob clob) { + if ( ext == null || ext.clobs == null ) { log.debug( "Request to release Clob, but appears no Clobs have ever been registered" ); return; } - clobs.remove( clob ); + ext.clobs.remove( clob ); } @Override - public void register(NClob nclob) { + public void register(final NClob nclob) { // todo : just store them in clobs? - if ( nclobs == null ) { - nclobs = new ArrayList<>(); - } - nclobs.add( nclob ); + getExtendedStateForWrite().registerNClob( nclob ); } @Override public void release(NClob nclob) { // todo : just store them in clobs? - if ( nclobs == null ) { + if ( ext == null || ext.nclobs == null ) { log.debug( "Request to release NClob, but appears no NClobs have ever been registered" ); return; } - nclobs.remove( nclob ); + ext.nclobs.remove( nclob ); } @Override @@ -323,7 +310,7 @@ public void cancelLastQuery() { @Override public void releaseResources() { - log.trace( "Releasing JDBC resources" ); + if ( IS_TRACE_ENABLED ) log.trace( "Releasing JDBC resources" ); if ( jdbcEventHandler != null ) { jdbcEventHandler.jdbcReleaseRegistryResourcesStart(); @@ -332,43 +319,8 @@ public void releaseResources() { xref.forEach( ResourceRegistryStandardImpl::releaseXref ); xref.clear(); - closeAll( unassociatedResultSets ); - - if ( blobs != null ) { - blobs.forEach( blob -> { - try { - blob.free(); - } - catch (SQLException e) { - log.debugf( "Unable to free JDBC Blob reference [%s]", e.getMessage() ); - } - } ); - //for these, it seems better to null the map rather than clear it: - blobs = null; - } - - if ( clobs != null ) { - clobs.forEach( clob -> { - try { - clob.free(); - } - catch (SQLException e) { - log.debugf( "Unable to free JDBC Clob reference [%s]", e.getMessage() ); - } - } ); - clobs = null; - } - - if ( nclobs != null ) { - nclobs.forEach( nclob -> { - try { - nclob.free(); - } - catch (SQLException e) { - log.debugf( "Unable to free JDBC NClob reference [%s]", e.getMessage() ); - } - } ); - nclobs = null; + if ( ext != null ) { + ext.releaseResources(); } if ( jdbcEventHandler != null ) { @@ -376,11 +328,109 @@ public void releaseResources() { } } - private boolean hasRegistered(final HashMap resource) { + private static boolean hasRegistered(final HashMap resource) { return resource != null && !resource.isEmpty(); } - private boolean hasRegistered(final ArrayList resource) { + private static boolean hasRegistered(final ArrayList resource) { return resource != null && !resource.isEmpty(); } + + /** + * Keeping this state separate from the main instance state as these are less-so commonly + * used: this keeps memory pressure low and the code a bit more organized. + * This implies that instances of ExtendedState should be lazily initialized, and access + * carefully guarded against null. + */ + private static class ExtendedState { + //All fields lazily initialized as well: + private HashMap unassociatedResultSets; + private ArrayList blobs; + private ArrayList clobs; + private ArrayList nclobs; + + public boolean hasRegisteredResources() { + return hasRegistered( unassociatedResultSets ) + || hasRegistered( blobs ) + || hasRegistered( clobs ) + || hasRegistered( nclobs ); + } + + public void releaseUnassociatedResult(final ResultSet resultSet) { + final Object removed = unassociatedResultSets == null ? null : unassociatedResultSets.remove( resultSet ); + if ( removed == null ) { + log.unregisteredResultSetWithoutStatement(); + } + } + + public void storeUnassociatedResultset(ResultSet resultSet) { + if ( unassociatedResultSets == null ) { + this.unassociatedResultSets = new HashMap<>(); + } + unassociatedResultSets.put( resultSet, PRESENT ); + } + + public void registerBlob(final Blob blob) { + if ( blobs == null ) { + blobs = new ArrayList<>(); + } + + blobs.add( blob ); + } + + public void registerClob(final Clob clob) { + if ( clobs == null ) { + clobs = new ArrayList<>(); + } + clobs.add( clob ); + } + + public void registerNClob(final NClob nclob) { + if ( nclobs == null ) { + nclobs = new ArrayList<>(); + } + nclobs.add( nclob ); + } + + public void releaseResources() { + closeAll( unassociatedResultSets ); + + if ( blobs != null ) { + blobs.forEach( blob -> { + try { + blob.free(); + } + catch (SQLException e) { + log.debugf( "Unable to free JDBC Blob reference [%s]", e.getMessage() ); + } + } ); + //for these, it seems better to null the map rather than clear it: + blobs = null; + } + + if ( clobs != null ) { + clobs.forEach( clob -> { + try { + clob.free(); + } + catch (SQLException e) { + log.debugf( "Unable to free JDBC Clob reference [%s]", e.getMessage() ); + } + } ); + clobs = null; + } + + if ( nclobs != null ) { + nclobs.forEach( nclob -> { + try { + nclob.free(); + } + catch (SQLException e) { + log.debugf( "Unable to free JDBC NClob reference [%s]", e.getMessage() ); + } + } ); + nclobs = null; + } + } + } }