Skip to content

Lighter registry standard impl #10464

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 2, 2025
Merged

Lighter registry standard impl #10464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0f16a82
Introduce a separate ExtendedState holder for ResourceRegistryStandar…
Sanne Jul 2, 2025
6791e42
Remove dead constants
Sanne Jul 2, 2025
69a5b86
Improve handling of trace logging in ResourceRegistryStandardImpl
Sanne Jul 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions hibernate-core/src/main/java/org/hibernate/query/sqm/SqmTreeTransformationLogger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,4 @@ public interface SqmTreeTransformationLogger {

Logger LOGGER = Logger.getLogger( LOGGER_NAME );

boolean TRACE_ENABLED = LOGGER.isTraceEnabled();
boolean DEBUG_ENABLED = LOGGER.isDebugEnabled();
}
220 changes: 135 additions & 85 deletions ...core/src/main/java/org/hibernate/resource/jdbc/internal/ResourceRegistryStandardImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -50,12 +51,8 @@ public final class ResourceRegistryStandardImpl implements ResourceRegistry {
private final JdbcEventHandler jdbcEventHandler;

private final HashMap<Statement, HashMap<ResultSet,Object>> xref = new HashMap<>();
private HashMap<ResultSet,Object> unassociatedResultSets;

private ArrayList<Blob> blobs;
private ArrayList<Clob> clobs;
private ArrayList<NClob> nclobs;

private ExtendedState ext;
private Statement lastQuery;

public ResourceRegistryStandardImpl() {
Expand All @@ -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<ResultSet,Object> previousValue = xref.putIfAbsent( statement, EMPTY );
if ( previousValue != null ) {
Expand All @@ -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<ResultSet,Object> resultSets = xref.remove( statement );
if ( resultSets != null ) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 );
Expand All @@ -164,7 +157,7 @@ private static void releaseXref(final Statement s, final HashMap<ResultSet, Obje
}

private static void close(final ResultSet resultSet) {
log.tracef( "Closing result set [%s]", resultSet );
if ( IS_TRACE_ENABLED ) log.tracef( "Closing result set [%s]", resultSet );

try {
if ( resultSet != null ) {
Expand All @@ -181,7 +174,7 @@ private static void close(final ResultSet resultSet) {
}

private static void close(Statement statement) {
log.tracef( "Closing prepared statement [%s]", statement );
if ( IS_TRACE_ENABLED ) log.tracef( "Closing prepared statement [%s]", statement );

try {
// if we are unable to "clean" the prepared statement,
Expand Down Expand Up @@ -215,7 +208,7 @@ private static void close(Statement statement) {

@Override
public void register(ResultSet resultSet, Statement statement) {
log.tracef( "Registering result set [%s]", resultSet );
if ( IS_TRACE_ENABLED ) log.tracef( "Registering result set [%s]", resultSet );

if ( statement == null ) {
try {
Expand All @@ -241,11 +234,15 @@ public void register(ResultSet resultSet, Statement statement) {
resultSets.put( resultSet, PRESENT );
}
else {
if ( unassociatedResultSets == null ) {
this.unassociatedResultSets = new 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) {
Expand All @@ -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
Expand All @@ -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();
Expand All @@ -332,55 +319,118 @@ 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 ) {
jdbcEventHandler.jdbcReleaseRegistryResourcesEnd();
}
}

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<ResultSet,Object> unassociatedResultSets;
private ArrayList<Blob> blobs;
private ArrayList<Clob> clobs;
private ArrayList<NClob> 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;
}
}
}
}