HSEARCH-3056 Avoid instantiating lambdas when calling chained Contain…

…erExtractors Signed-off-by: Yoann Rodière <yoann@hibernate.org>
hibernate · Nov 2, 2020 · 90df91c · 90df91c
1 parent 6ce452d
commit 90df91c
Show file tree

Hide file tree

Showing 9 changed files with 180 additions and 95 deletions.
diff --git a/...apper/pojo/automaticindexing/impl/PojoImplicitReindexingResolverContainerElementNode.java b/...apper/pojo/automaticindexing/impl/PojoImplicitReindexingResolverContainerElementNode.java
@@ -29,17 +29,17 @@ public class PojoImplicitReindexingResolverContainerElementNode<C, S, V>
 
 	private final ContainerExtractorHolder<C, V> extractorHolder;
 	private final PojoImplicitReindexingResolverNode<? super V, S> nested;
-	private final ValueProcessor<PojoReindexingCollector, V, PojoImplicitReindexingResolverRootContext<S>> perValueDelegate;
+	private final ValueProcessor<PojoReindexingCollector, ? super C, PojoImplicitReindexingResolverRootContext<S>> extractingDelegate;
 
 	public PojoImplicitReindexingResolverContainerElementNode(ContainerExtractorHolder<C, V> extractorHolder,
 			PojoImplicitReindexingResolverNode<? super V, S> nested) {
 		this.extractorHolder = extractorHolder;
 		this.nested = nested;
-		this.perValueDelegate = (collector, value, context) -> {
+		this.extractingDelegate = extractorHolder.wrap( (collector, value, context) -> {
 			if ( value != null ) {
 				nested.resolveEntitiesToReindex( collector, value, context );
 			}
-		};
+		} );
 	}
 
 	@Override
@@ -53,14 +53,14 @@ public void close() {
 	@Override
 	public void appendTo(ToStringTreeBuilder builder) {
 		builder.attribute( "operation", "process container element" );
-		builder.attribute( "extractor", extractorHolder.get() );
+		builder.attribute( "extractor", extractorHolder );
 		builder.attribute( "nested", nested );
 	}
 
 	@Override
 	public void resolveEntitiesToReindex(PojoReindexingCollector collector,
 			C dirty, PojoImplicitReindexingResolverRootContext<S> context) {
-		extractorHolder.get().extract( dirty, perValueDelegate, collector, context );
+		extractingDelegate.process( collector, dirty, context );
 	}
 
 }
diff --git a/...main/java/org/hibernate/search/mapper/pojo/extractor/impl/ChainingContainerExtractor.java b/...main/java/org/hibernate/search/mapper/pojo/extractor/impl/ChainingContainerExtractor.java
diff --git a/...ava/org/hibernate/search/mapper/pojo/extractor/impl/ChainingContainerExtractorHolder.java b/...ava/org/hibernate/search/mapper/pojo/extractor/impl/ChainingContainerExtractorHolder.java
@@ -0,0 +1,56 @@
+/*
+ * Hibernate Search, full-text search for your domain model
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.search.mapper.pojo.extractor.impl;
+
+import org.hibernate.search.engine.environment.bean.BeanHolder;
+import org.hibernate.search.mapper.pojo.extractor.ContainerExtractor;
+import org.hibernate.search.mapper.pojo.extractor.ValueProcessor;
+import org.hibernate.search.util.common.impl.Closer;
+
+final class ChainingContainerExtractorHolder<C, U, V> implements ContainerExtractorHolder<C, V> {
+	private final ContainerExtractorHolder<C, U> base;
+	private final BeanHolder<? extends ContainerExtractor<? super U, V>> chained;
+
+	public ChainingContainerExtractorHolder(ContainerExtractorHolder<C, U> base,
+			BeanHolder<? extends ContainerExtractor<? super U, V>> chained) {
+		this.base = base;
+		this.chained = chained;
+	}
+
+	@Override
+	public String toString() {
+		StringBuilder builder = new StringBuilder( "[" );
+		appendToString( builder );
+		builder.append( "]" );
+		return builder.toString();
+	}
+
+	@Override
+	public void close() {
+		try ( Closer<RuntimeException> closer = new Closer<>() ) {
+			closer.push( ContainerExtractorHolder::close, base );
+			closer.push( BeanHolder::close, chained );
+		}
+	}
+
+	@Override
+	public <T, C2> ValueProcessor<T, C, C2> wrap(ValueProcessor<T, ? super V, C2> perValueProcessor) {
+		return base.wrap( new ContainerExtractingProcessor<>( chained.get(), perValueProcessor ) );
+	}
+
+	@Override
+	public boolean multiValued() {
+		return base.multiValued() || chained.get().multiValued();
+	}
+
+	@Override
+	public void appendToString(StringBuilder builder) {
+		base.appendToString( builder );
+		builder.append( ", " );
+		builder.append( chained );
+	}
+}
diff --git a/...in/java/org/hibernate/search/mapper/pojo/extractor/impl/ContainerExtractingProcessor.java b/...in/java/org/hibernate/search/mapper/pojo/extractor/impl/ContainerExtractingProcessor.java
@@ -0,0 +1,34 @@
+/*
+ * Hibernate Search, full-text search for your domain model
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.search.mapper.pojo.extractor.impl;
+
+import org.hibernate.search.mapper.pojo.extractor.ContainerExtractor;
+import org.hibernate.search.mapper.pojo.extractor.ValueProcessor;
+
+public final class ContainerExtractingProcessor<T, C, V, C2> implements ValueProcessor<T, C, C2> {
+	private final ContainerExtractor<? super C, V> extractor;
+	private final ValueProcessor<T, ? super V, C2> perValueProcessor;
+
+	public ContainerExtractingProcessor(ContainerExtractor<? super C, V> extractor,
+			ValueProcessor<T, ? super V, C2> perValueProcessor) {
+		this.extractor = extractor;
+		this.perValueProcessor = perValueProcessor;
+	}
+
+	@Override
+	public String toString() {
+		return "ContainerExtractingProcessor["
+				+ "extractor=" + extractor
+				+ ", perValueProcessor=" + perValueProcessor
+				+ "]";
+	}
+
+	@Override
+	public void process(T target, C container, C2 context) {
+		extractor.extract( container, perValueProcessor, target, context );
+	}
+}
diff --git a/...c/main/java/org/hibernate/search/mapper/pojo/extractor/impl/ContainerExtractorBinder.java b/...c/main/java/org/hibernate/search/mapper/pojo/extractor/impl/ContainerExtractorBinder.java
@@ -155,7 +155,7 @@ public <C, V> ContainerExtractorHolder<C, V> create(BoundContainerExtractorPath<
 					"Received a request to create extractors, but the extractor path was empty."
 			);
 		}
-		ContainerExtractor<? super C, ?> extractor = null;
+		ContainerExtractorHolder<C, ?> extractorHolder = null;
 		List<BeanHolder<?>> beanHolders = new ArrayList<>();
 		try {
 			for ( String extractorName : boundPath.getExtractorPath().explicitExtractorNames() ) {
@@ -164,17 +164,23 @@ public <C, V> ContainerExtractorHolder<C, V> create(BoundContainerExtractorPath<
 				BeanHolder<? extends ContainerExtractor> newExtractorHolder =
 						beanResolver.resolve( extractorClass );
 				beanHolders.add( newExtractorHolder );
-				if ( extractor == null ) {
-					// First extractor: must be able to process type C
-					extractor = (ContainerExtractor<? super C, ?>) newExtractorHolder.get();
+				if ( extractorHolder == null ) {
+					// The use of a raw type is fine here:
+					// - This is the first extractor, so we know from previous reflection checks that it accepts type C
+					// - The BeanHolder's get() method, by contract, always returns the same instance,
+					//   so we know the returned extractor will always return values of the same type V.
+					extractorHolder = new SingleContainerExtractorHolder<>( (BeanHolder) newExtractorHolder );
 				}
 				else {
-					extractor = new ChainingContainerExtractor( extractor, newExtractorHolder.get() );
+					// The use of a raw type is fine here:
+					// - The BeanHolder's get() method, by contract, always returns the same instance,
+					//   so we know the returned extractor will always return values of the same type V.
+					extractorHolder = new ChainingContainerExtractorHolder<>( extractorHolder,
+							(BeanHolder) newExtractorHolder );
 				}
 			}
-			return new ContainerExtractorHolder<>(
-					(ContainerExtractor<? super C, V>) extractor, beanHolders
-			);
+			// Final extractor: must return values of type V
+			return (ContainerExtractorHolder<C, V>) extractorHolder;
 		}
 		catch (RuntimeException e) {
 			new SuppressingCloser( e ).pushAll( BeanHolder::close, beanHolders );

diff --git a/...c/main/java/org/hibernate/search/mapper/pojo/extractor/impl/ContainerExtractorHolder.java b/...c/main/java/org/hibernate/search/mapper/pojo/extractor/impl/ContainerExtractorHolder.java
@@ -6,30 +6,26 @@
  */
 package org.hibernate.search.mapper.pojo.extractor.impl;
 
-import java.util.List;
-
-import org.hibernate.search.engine.environment.bean.BeanHolder;
 import org.hibernate.search.mapper.pojo.extractor.ContainerExtractor;
-import org.hibernate.search.util.common.impl.Closer;
-
-public class ContainerExtractorHolder<C, V> implements AutoCloseable {
-	private final ContainerExtractor<? super C, V> chain;
-	private final List<BeanHolder<?>> chainElementBeanHolders;
+import org.hibernate.search.mapper.pojo.extractor.ValueProcessor;
 
-	ContainerExtractorHolder(ContainerExtractor<? super C, V> chain,
-			List<BeanHolder<?>> chainElementBeanHolders) {
-		this.chain = chain;
-		this.chainElementBeanHolders = chainElementBeanHolders;
-	}
+public interface ContainerExtractorHolder<C, V> extends AutoCloseable {
 
 	@Override
-	public void close() {
-		try ( Closer<RuntimeException> closer = new Closer<>() ) {
-			closer.pushAll( BeanHolder::close, chainElementBeanHolders );
-		}
-	}
+	void close();
+
+	/**
+	 * @param perValueProcessor A processor for values extracted from the container.
+	 * @return A processor that accepts a container,
+	 * extracts values from the given container and passes each value to the given
+	 * {@code perValueProcessor}.
+	 */
+	<T, C2> ValueProcessor<T, C, C2> wrap(ValueProcessor<T, ? super V, C2> perValueProcessor);
+
+	/**
+	 * @return See {@link ContainerExtractor#multiValued()}.
+	 */
+	boolean multiValued();
 
-	public ContainerExtractor<? super C, V> get() {
-		return chain;
-	}
+	void appendToString(StringBuilder builder);
 }
diff --git a/.../java/org/hibernate/search/mapper/pojo/extractor/impl/SingleContainerExtractorHolder.java b/.../java/org/hibernate/search/mapper/pojo/extractor/impl/SingleContainerExtractorHolder.java
@@ -0,0 +1,48 @@
+/*
+ * Hibernate Search, full-text search for your domain model
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.search.mapper.pojo.extractor.impl;
+
+
+import org.hibernate.search.engine.environment.bean.BeanHolder;
+import org.hibernate.search.mapper.pojo.extractor.ContainerExtractor;
+import org.hibernate.search.mapper.pojo.extractor.ValueProcessor;
+
+final class SingleContainerExtractorHolder<C, V> implements ContainerExtractorHolder<C, V> {
+
+	private final BeanHolder<? extends ContainerExtractor<? super C, V>> extractorBeanHolder;
+
+	SingleContainerExtractorHolder(BeanHolder<? extends ContainerExtractor<? super C, V>> extractorBeanHolder) {
+		this.extractorBeanHolder = extractorBeanHolder;
+	}
+
+	@Override
+	public String toString() {
+		StringBuilder builder = new StringBuilder();
+		appendToString( builder );
+		return builder.toString();
+	}
+
+	@Override
+	public void close() {
+		extractorBeanHolder.close();
+	}
+
+	@Override
+	public <T, C2> ValueProcessor<T, C, C2> wrap(ValueProcessor<T, ? super V, C2> perValueProcessor) {
+		return new ContainerExtractingProcessor<>( extractorBeanHolder.get(), perValueProcessor );
+	}
+
+	@Override
+	public boolean multiValued() {
+		return extractorBeanHolder.get().multiValued();
+	}
+
+	@Override
+	public void appendToString(StringBuilder builder) {
+		builder.append( extractorBeanHolder.get() );
+	}
+}
diff --git a/...apper/pojo/processing/building/impl/PojoIndexingProcessorContainerElementNodeBuilder.java b/...apper/pojo/processing/building/impl/PojoIndexingProcessorContainerElementNodeBuilder.java
@@ -43,7 +43,7 @@ class PojoIndexingProcessorContainerElementNodeBuilder<P extends C, C, V> extend
 		valueNodeProcessorCollectionBuilder = new PojoIndexingProcessorValueNodeBuilderDelegate<>(
 				modelPath,
 				mappingHelper, bindingContext,
-				extractorHolder.get().multiValued()
+				extractorHolder.multiValued()
 		);
 	}
 

diff --git a/...bernate/search/mapper/pojo/processing/impl/PojoIndexingProcessorContainerElementNode.java b/...bernate/search/mapper/pojo/processing/impl/PojoIndexingProcessorContainerElementNode.java
@@ -24,13 +24,14 @@ public class PojoIndexingProcessorContainerElementNode<C, V> extends PojoIndexin
 
 	private final ContainerExtractorHolder<C, V> extractorHolder;
 	private final PojoIndexingProcessor<? super V> nested;
-	private final ValueProcessor<DocumentElement, V, PojoIndexingProcessorSessionContext> perValueDelegate;
+	private final ValueProcessor<DocumentElement, ? super C, PojoIndexingProcessorSessionContext> extractingDelegate;
 
 	public PojoIndexingProcessorContainerElementNode(ContainerExtractorHolder<C, V> extractorHolder,
 			PojoIndexingProcessor<? super V> nested) {
 		this.extractorHolder = extractorHolder;
 		this.nested = nested;
-		this.perValueDelegate = (target, value, sessionContext) -> nested.process( target, value, sessionContext );
+		this.extractingDelegate = extractorHolder.wrap( (target, value, sessionContext) ->
+				nested.process( target, value, sessionContext ) );
 	}
 
 	@Override
@@ -44,12 +45,12 @@ public void close() {
 	@Override
 	public void appendTo(ToStringTreeBuilder builder) {
 		builder.attribute( "operation", "process container element" );
-		builder.attribute( "extractor", extractorHolder.get() );
+		builder.attribute( "extractor", extractorHolder );
 		builder.attribute( "nested", nested );
 	}
 
 	@Override
 	public final void process(DocumentElement target, C source, PojoIndexingProcessorSessionContext sessionContext) {
-		extractorHolder.get().extract( source, perValueDelegate, target, sessionContext );
+		extractingDelegate.process( target, source, sessionContext );
 	}
 }