From 9bfffd85d7beeaee98630bb73dbbb3fb7a11f5e7 Mon Sep 17 00:00:00 2001 From: Sanne Grinovero Date: Wed, 21 Aug 2019 11:48:18 +0100 Subject: [PATCH] HHH-13565 Ensure all events from EventListenerGroup can be fired without allocations --- .../internal/EventListenerGroupImpl.java | 95 +++++++++++++++---- .../service/spi/EventActionWithParameter.java | 17 ++++ .../event/service/spi/EventListenerGroup.java | 13 +++ .../org/hibernate/internal/SessionImpl.java | 12 +-- 4 files changed, 112 insertions(+), 25 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/event/service/spi/EventActionWithParameter.java diff --git a/hibernate-core/src/main/java/org/hibernate/event/service/internal/EventListenerGroupImpl.java b/hibernate-core/src/main/java/org/hibernate/event/service/internal/EventListenerGroupImpl.java index 20e9d5c40a..3f4cc95fd7 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/service/internal/EventListenerGroupImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/event/service/internal/EventListenerGroupImpl.java @@ -6,15 +6,19 @@ */ package org.hibernate.event.service.internal; +import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashSet; +import java.util.List; import java.util.ListIterator; +import java.util.Map; import java.util.Set; import java.util.function.BiConsumer; import java.util.function.Supplier; import org.hibernate.event.service.spi.DuplicationStrategy; +import org.hibernate.event.service.spi.EventActionWithParameter; import org.hibernate.event.service.spi.EventListenerGroup; import org.hibernate.event.service.spi.EventListenerRegistrationException; import org.hibernate.event.service.spi.JpaBootstrapSensitive; @@ -23,6 +27,7 @@ /** * @author Steve Ebersole + * @author Sanne Grinovero */ class EventListenerGroupImpl implements EventListenerGroup { private EventType eventType; @@ -30,9 +35,14 @@ class EventListenerGroupImpl implements EventListenerGroup { private final Set duplicationStrategies = new LinkedHashSet<>(); - // Performance: make sure a forEach iteration on this type is efficient; in particular we choose ArrayList - // so to avoid allocating iterators. - private ArrayList listeners; + // Performance: make sure iteration on this type is efficient; in particular we do not want to allocate iterators, + // not having to capture state in lambdas. + // So we keep the listeners in both a List (for convenience) and in an array (for iteration). Make sure + // their content stays in synch! + private T[] listeners = null; + + //Update both fields when making changes! + private List listenersAsList; public EventListenerGroupImpl(EventType eventType, EventListenerRegistryImpl listenerRegistry) { this.eventType = eventType; @@ -66,7 +76,8 @@ public boolean isEmpty() { @Override public int count() { - return listeners == null ? 0 : listeners.size(); + final T[] ls = listeners; + return ls == null ? 0 : ls.length; } @Override @@ -74,16 +85,16 @@ public void clear() { if ( duplicationStrategies != null ) { duplicationStrategies.clear(); } - if ( listeners != null ) { - listeners.clear(); - } + listeners = null; + listenersAsList = null; } @Override public final void fireLazyEventOnEachListener(final Supplier eventSupplier, final BiConsumer actionOnEvent) { - if ( listeners != null && listeners.size() != 0 ) { + final T[] ls = listeners; + if ( ls != null && ls.length != 0 ) { final U event = eventSupplier.get(); - for ( T listener : this.listeners ) { + for ( T listener : ls ) { actionOnEvent.accept( listener, event ); } } @@ -91,13 +102,24 @@ public final void fireLazyEventOnEachListener(final Supplier eventSupplie @Override public final void fireEventOnEachListener(final U event, final BiConsumer actionOnEvent) { - if ( listeners != null ) { - for ( T listener : this.listeners ) { + final T[] ls = listeners; + if ( ls != null ) { + for ( T listener : ls ) { actionOnEvent.accept( listener, event ); } } } + @Override + public void fireEventOnEachListener(final U event, final X parameter, final EventActionWithParameter actionOnEvent) { + final T[] ls = listeners; + if ( ls != null ) { + for ( T listener : ls ) { + actionOnEvent.applyEventToListener( listener, event, parameter ); + } + } + } + @Override public void addDuplicationStrategy(DuplicationStrategy strategy) { duplicationStrategies.add( strategy ); @@ -105,22 +127,44 @@ public void addDuplicationStrategy(DuplicationStrategy strategy) { /** * Implementation note: should be final for performance reasons. + * @deprecated this is not the most efficient way for iterating the event listeners. + * See {@link #fireEventOnEachListener(Object, BiConsumer)} and co. for better alternatives. */ @Override + @Deprecated public final Iterable listeners() { - return listeners == null ? Collections.EMPTY_LIST : listeners; + final List ls = listenersAsList; + return ls == null ? Collections.EMPTY_LIST : ls; } @Override @SafeVarargs public final void appendListeners(T... listeners) { + internalAppendListeners( listeners ); + checkForArrayRefresh(); + } + + private void checkForArrayRefresh() { + final List list = listenersAsList; + if ( this.listeners == null ) { + T[] a = (T[]) Array.newInstance( eventType.baseListenerInterface(), list.size() ); + listeners = list.toArray( a ); + } + } + + private void internalAppendListeners(T[] listeners) { for ( T listener : listeners ) { - appendListener( listener ); + internalAppendListener( listener ); } } @Override public void appendListener(T listener) { + internalAppendListener( listener ); + checkForArrayRefresh(); + } + + private void internalAppendListener(T listener) { if ( listenerShouldGetAdded( listener ) ) { internalAppend( listener ); } @@ -129,28 +173,39 @@ public void appendListener(T listener) { @Override @SafeVarargs public final void prependListeners(T... listeners) { + internalPrependListeners( listeners ); + checkForArrayRefresh(); + } + + private void internalPrependListeners(T[] listeners) { for ( T listener : listeners ) { - prependListener( listener ); + internalPreprendListener( listener ); } } @Override public void prependListener(T listener) { + internalPreprendListener( listener ); + checkForArrayRefresh(); + } + + private void internalPreprendListener(T listener) { if ( listenerShouldGetAdded( listener ) ) { internalPrepend( listener ); } } private boolean listenerShouldGetAdded(T listener) { - if ( listeners == null ) { - listeners = new ArrayList<>(); + final List ts = listenersAsList; + if ( ts == null ) { + listenersAsList = new ArrayList<>(); return true; // no need to do de-dup checks } boolean doAdd = true; strategy_loop: for ( DuplicationStrategy strategy : duplicationStrategies ) { - final ListIterator itr = listeners.listIterator(); + final ListIterator itr = ts.listIterator(); while ( itr.hasNext() ) { final T existingListener = itr.next(); if ( strategy.areMatch( listener, existingListener ) ) { @@ -180,7 +235,8 @@ private boolean listenerShouldGetAdded(T listener) { private void internalPrepend(T listener) { checkAgainstBaseInterface( listener ); performInjections( listener ); - listeners.add( 0, listener ); + listenersAsList.add( 0, listener ); + listeners = null; //Marks it for refreshing } private void performInjections(T listener) { @@ -206,6 +262,7 @@ private void checkAgainstBaseInterface(T listener) { private void internalAppend(T listener) { checkAgainstBaseInterface( listener ); performInjections( listener ); - listeners.add( listener ); + listenersAsList.add( listener ); + listeners = null; //Marks it for refreshing } } diff --git a/hibernate-core/src/main/java/org/hibernate/event/service/spi/EventActionWithParameter.java b/hibernate-core/src/main/java/org/hibernate/event/service/spi/EventActionWithParameter.java new file mode 100644 index 0000000000..e685b5293b --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/event/service/spi/EventActionWithParameter.java @@ -0,0 +1,17 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.event.service.spi; + +import org.hibernate.Incubating; + +@Incubating +@FunctionalInterface +public interface EventActionWithParameter { + + void applyEventToListener(T eventListener, U action, X param); + +} diff --git a/hibernate-core/src/main/java/org/hibernate/event/service/spi/EventListenerGroup.java b/hibernate-core/src/main/java/org/hibernate/event/service/spi/EventListenerGroup.java index 1f4e823a84..213f6acb64 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/service/spi/EventListenerGroup.java +++ b/hibernate-core/src/main/java/org/hibernate/event/service/spi/EventListenerGroup.java @@ -7,9 +7,11 @@ package org.hibernate.event.service.spi; import java.io.Serializable; +import java.util.Map; import java.util.function.BiConsumer; import java.util.function.Supplier; +import org.hibernate.Incubating; import org.hibernate.event.spi.EventType; /** @@ -35,6 +37,12 @@ public interface EventListenerGroup extends Serializable { public int count(); + /** + * @deprecated this is not the most efficient way for iterating the event listeners. + * See {@link #fireEventOnEachListener(Object, BiConsumer)} and its overloaded variants for better alternatives. + * @return + */ + @Deprecated public Iterable listeners(); /** @@ -67,6 +75,7 @@ public interface EventListenerGroup extends Serializable { * @param actionOnEvent * @param the kind of event */ + @Incubating void fireLazyEventOnEachListener(final Supplier eventSupplier, final BiConsumer actionOnEvent); /** @@ -76,6 +85,10 @@ public interface EventListenerGroup extends Serializable { * @param actionOnEvent * @param the kind of event */ + @Incubating void fireEventOnEachListener(final U event, final BiConsumer actionOnEvent); + @Incubating + void fireEventOnEachListener(final U event, X param, final EventActionWithParameter actionOnEvent); + } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java index c393a8ce53..656feaa482 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java @@ -724,7 +724,7 @@ private void firePersist(final Map copiedAlready, final PersistEvent event) { try { //Uses a capturing lambda in this case as we need to carry the additional Map parameter: fastSessionServices.eventListenerGroup_PERSIST - .fireEventOnEachListener( event, (l, e) -> l.onPersist( e, copiedAlready ) ); + .fireEventOnEachListener( event, copiedAlready, PersistEventListener::onPersist ); } catch ( MappingException e ) { throw getExceptionConverter().convert( new IllegalArgumentException( e.getMessage() ) ) ; @@ -745,7 +745,7 @@ public void persistOnFlush(String entityName, Object object, Map copiedAlready) checkOpenOrWaitingForAutoClose(); pulseTransactionCoordinator(); PersistEvent event = new PersistEvent( entityName, object, this ); - fastSessionServices.eventListenerGroup_PERSIST_ONFLUSH.fireEventOnEachListener( event, (l,e) -> l.onPersist( e, copiedAlready ) ); + fastSessionServices.eventListenerGroup_PERSIST_ONFLUSH.fireEventOnEachListener( event, copiedAlready, PersistEventListener::onPersist ); delayedAfterCompletion(); } @@ -793,7 +793,7 @@ private Object fireMerge(MergeEvent event) { private void fireMerge(final Map copiedAlready, final MergeEvent event) { try { pulseTransactionCoordinator(); - fastSessionServices.eventListenerGroup_MERGE.fireEventOnEachListener( event, (l,e) -> l.onMerge( e, copiedAlready ) ); + fastSessionServices.eventListenerGroup_MERGE.fireEventOnEachListener( event, copiedAlready, MergeEventListener::onMerge ); } catch ( ObjectDeletedException sse ) { throw getExceptionConverter().convert( new IllegalArgumentException( sse ) ); @@ -904,7 +904,7 @@ private void fireDelete(final DeleteEvent event) { private void fireDelete(final DeleteEvent event, final Set transientEntities) { try{ pulseTransactionCoordinator(); - fastSessionServices.eventListenerGroup_DELETE.fireEventOnEachListener( event, (l,e) -> l.onDelete( e, transientEntities ) ); + fastSessionServices.eventListenerGroup_DELETE.fireEventOnEachListener( event, transientEntities, DeleteEventListener::onDelete ); } catch ( ObjectDeletedException sse ) { throw getExceptionConverter().convert( new IllegalArgumentException( sse ) ); @@ -1179,7 +1179,7 @@ private void fireLoad(LoadEvent event, LoadType loadType) { // it seems they prevent these hot methods from being inlined. private void fireLoadNoChecks(final LoadEvent event, final LoadType loadType) { pulseTransactionCoordinator(); - fastSessionServices.eventListenerGroup_LOAD.fireEventOnEachListener( event, (l,e) -> l.onLoad( e, loadType ) ); + fastSessionServices.eventListenerGroup_LOAD.fireEventOnEachListener( event, loadType, LoadEventListener::onLoad ); } private void fireResolveNaturalId(final ResolveNaturalIdEvent event) { @@ -1262,7 +1262,7 @@ private void fireRefresh(final RefreshEvent event) { private void fireRefresh(final Map refreshedAlready, final RefreshEvent event) { try { pulseTransactionCoordinator(); - fastSessionServices.eventListenerGroup_REFRESH.fireEventOnEachListener( event, (l,e) -> l.onRefresh( e, refreshedAlready ) ); + fastSessionServices.eventListenerGroup_REFRESH.fireEventOnEachListener( event, refreshedAlready, RefreshEventListener::onRefresh ); } catch (RuntimeException e) { throw getExceptionConverter().convert( e );