aries-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rmannibu...@apache.org
Subject [aries-cdi] 03/03: [ARIES-2020] basic Discovery optimizations
Date Fri, 30 Oct 2020 10:20:52 GMT
This is an automated email from the ASF dual-hosted git repository.

rmannibucau pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/aries-cdi.git

commit 2d071cbc932da248f56d937aa913890870c69d70
Author: Romain Manni-Bucau <rmannibucau@gmail.com>
AuthorDate: Fri Oct 30 11:20:34 2020 +0100

    [ARIES-2020] basic Discovery optimizations
---
 .../internal/annotated/AnnotatedImpl.java          | 26 ++++++-
 .../internal/annotated/CachingAnnotated.java       | 29 ++++++++
 .../container/internal/container/Discovery.java    | 44 ++++++-----
 .../cdi/container/internal/util/Annotates.java     | 85 ++++++++++++++--------
 .../aries/cdi/container/internal/util/Maps.java    | 19 -----
 .../cdi/container/internal/util/AnnotatesTest.java | 39 ++++++++++
 .../org/apache/aries/cdi/test/cases/TrimTests.java |  6 +-
 .../java/org/apache/aries/cdi/test/tb17/A.java     |  1 -
 8 files changed, 172 insertions(+), 77 deletions(-)

diff --git a/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/annotated/AnnotatedImpl.java
b/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/annotated/AnnotatedImpl.java
index 463fc05..f7c498b 100644
--- a/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/annotated/AnnotatedImpl.java
+++ b/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/annotated/AnnotatedImpl.java
@@ -20,18 +20,22 @@ import java.lang.annotation.Annotation;
 import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.Type;
 import java.util.Arrays;
+import java.util.List;
 import java.util.Set;
 
 import javax.enterprise.inject.spi.Annotated;
 
 import org.apache.aries.cdi.container.internal.util.Reflection;
 
-public class AnnotatedImpl<X> implements Annotated {
+public class AnnotatedImpl<X> implements CachingAnnotated {
 
 	private final Type _baseType;
 	private final AnnotatedElement _annotatedElement;
 	private final Set<Type> _typeClosure;
 
+	private Class<? extends Annotation> beanScope;
+	private List<Annotation> collectedAnnotations;
+
 	public AnnotatedImpl(final Type baseType, final AnnotatedElement annotatedElement) {
 		_baseType = baseType;
 		_annotatedElement = annotatedElement;
@@ -39,6 +43,26 @@ public class AnnotatedImpl<X> implements Annotated {
 	}
 
 	@Override
+	public List<Annotation> getCollectedAnnotations() {
+		return collectedAnnotations;
+	}
+
+	@Override
+	public void setCollectedAnnotations(final List<Annotation> collectedAnnotations) {
+		this.collectedAnnotations = collectedAnnotations;
+	}
+
+	@Override
+	public Class<? extends Annotation> getBeanScope() {
+		return beanScope;
+	}
+
+	@Override
+	public void setBeanScope(Class<? extends Annotation> beanScope) {
+		this.beanScope = beanScope;
+	}
+
+	@Override
 	public Type getBaseType() {
 		return _baseType;
 	}
diff --git a/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/annotated/CachingAnnotated.java
b/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/annotated/CachingAnnotated.java
new file mode 100644
index 0000000..9de0236
--- /dev/null
+++ b/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/annotated/CachingAnnotated.java
@@ -0,0 +1,29 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aries.cdi.container.internal.annotated;
+
+import javax.enterprise.inject.spi.Annotated;
+import java.lang.annotation.Annotation;
+import java.util.List;
+
+// because Discovery run should be free and not cost as much as a CDI starts we try to reduce
its cost by caching meta
+public interface CachingAnnotated extends Annotated {
+    // tested early and generally later too so cache it
+    Class<? extends Annotation> getBeanScope();
+    void setBeanScope(Class<? extends Annotation> value);
+
+    // used in all "tests" so worth a cache
+    List<Annotation> getCollectedAnnotations();
+    void setCollectedAnnotations(List<Annotation> annotations);
+}
diff --git a/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/container/Discovery.java
b/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/container/Discovery.java
index 07f56b6..71f8d18 100644
--- a/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/container/Discovery.java
+++ b/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/container/Discovery.java
@@ -20,6 +20,7 @@ import java.lang.reflect.Type;
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -136,23 +137,24 @@ public class Discovery {
 			}
 
 			try {
-				String beanName = Annotates.beanName(annotatedType);
-				Class<? extends Annotation> beanScope = Annotates.beanScope(annotatedType);
-				Map<String, Object> componentProperties = Annotates.componentProperties(annotatedType);
-				ServiceScope serviceScope = Annotates.serviceScope(annotatedType);
-				List<String> serviceTypes = Annotates.serviceClassNames(annotatedType);
-
 				if (annotatedType.isAnnotationPresent(SingleComponent.class)) {
-					doFactoryOrSingleComponent(osgiBean, osgiBean.getBeanClass(), annotatedType, beanName,
serviceTypes, serviceScope, componentProperties, ComponentType.SINGLE);
+					doFactoryOrSingleComponent(
+							osgiBean, osgiBean.getBeanClass(), annotatedType, Annotates.beanName(annotatedType),
+							Annotates.serviceClassNames(annotatedType), Annotates.serviceScope(annotatedType),
+							Annotates.componentProperties(annotatedType), ComponentType.SINGLE);
 				}
 				else if (annotatedType.isAnnotationPresent(FactoryComponent.class)) {
-					doFactoryOrSingleComponent(osgiBean, osgiBean.getBeanClass(), annotatedType, beanName,
serviceTypes, serviceScope, componentProperties, ComponentType.FACTORY);
+					doFactoryOrSingleComponent(
+							osgiBean, osgiBean.getBeanClass(), annotatedType, Annotates.beanName(annotatedType),
+							Annotates.serviceClassNames(annotatedType), Annotates.serviceScope(annotatedType),
+							Annotates.componentProperties(annotatedType), ComponentType.FACTORY);
 				}
 				else if (annotatedType.isAnnotationPresent(ComponentScoped.class)) {
 					_componentScoped.add(osgiBean);
 				}
 				else {
-					discoverActivations(osgiBean, osgiBean.getBeanClass(), annotatedType, null, beanScope,
serviceTypes, serviceScope, componentProperties);
+					discoverActivations(osgiBean, osgiBean.getBeanClass(), annotatedType, null,
+							Annotates.beanScope(annotatedType), Annotates.serviceClassNames(annotatedType));
 				}
 			}
 			catch (Exception e) {
@@ -174,31 +176,25 @@ public class Discovery {
 			annotatedType.getFields().stream().filter(this::isProduces).forEach(
 				annotatedField -> {
 					Class<? extends Annotation> beanScope = Annotates.beanScope(annotatedField);
-					Map<String, Object> componentProperties = Annotates.componentProperties(annotatedField);
-					ServiceScope serviceScope = Annotates.serviceScope(annotatedField);
 					List<String> serviceTypes = Annotates.serviceClassNames(annotatedField);
-
-					discoverActivations(osgiBean, osgiBean.getBeanClass(), annotatedField, annotatedField,
beanScope, serviceTypes, serviceScope, componentProperties);
+					discoverActivations(osgiBean, osgiBean.getBeanClass(), annotatedField, annotatedField,
beanScope, serviceTypes);
 				}
 			);
 
-			annotatedType.getMethods().stream().forEach(annotatedMethod -> {
+			annotatedType.getMethods().forEach(annotatedMethod -> {
 				if (isInjectOrProduces(annotatedMethod)) {
-					annotatedMethod.getParameters().stream().forEach(
+					annotatedMethod.getParameters().forEach(
 						annotatedParameter -> processAnnotated(annotatedParameter, annotatedParameter.getBaseType(),
Annotates.qualifiers(annotatedParameter), osgiBean)
 					);
 
 					if (isProduces(annotatedMethod)) {
 						Class<? extends Annotation> beanScope = Annotates.beanScope(annotatedMethod);
-						Map<String, Object> componentProperties = Annotates.componentProperties(annotatedMethod);
-						ServiceScope serviceScope = Annotates.serviceScope(annotatedMethod);
 						List<String> serviceTypes = Annotates.serviceClassNames(annotatedMethod);
-
-						discoverActivations(osgiBean, osgiBean.getBeanClass(), annotatedMethod, annotatedMethod,
beanScope, serviceTypes, serviceScope, componentProperties);
+						discoverActivations(osgiBean, osgiBean.getBeanClass(), annotatedMethod, annotatedMethod,
beanScope, serviceTypes);
 					}
 				}
 				else if (isDisposeOrObserves(annotatedMethod)) {
-					annotatedMethod.getParameters().subList(1, annotatedMethod.getParameters().size()).stream().forEach(
+					annotatedMethod.getParameters().stream().skip(1).forEach(
 						annotatedParameter -> processAnnotated(annotatedParameter, annotatedParameter.getBaseType(),
Annotates.qualifiers(annotatedParameter), osgiBean)
 					);
 				}
@@ -311,7 +307,9 @@ public class Discovery {
 		}
 	}
 
-	void discoverActivations(OSGiBean osgiBean, Class<?> declaringClass, Annotated annotated,
AnnotatedMember<?> producer, Class<? extends Annotation> scope, List<String>
serviceTypeNames, ServiceScope serviceScope, Map<String, Object> componentProperties)
{
+	void discoverActivations(OSGiBean osgiBean, Class<?> declaringClass, final Annotated
component,
+							 AnnotatedMember<?> producer, Class<? extends Annotation> scope,
+							 List<String> serviceTypeNames) {
 		String className = declaringClass.getName();
 
 		if (!_containerTemplate.beans.contains(className)) {
@@ -335,8 +333,8 @@ public class Discovery {
 			activationTemplate.cdiScope = scope;
 			activationTemplate.declaringClass = declaringClass;
 			activationTemplate.producer = producer;
-			activationTemplate.properties = componentProperties;
-			activationTemplate.scope = serviceScope;
+			activationTemplate.properties = Annotates.componentProperties(component);
+			activationTemplate.scope = Annotates.serviceScope(component);
 			activationTemplate.serviceClasses = serviceTypeNames;
 
 			_containerTemplate.activations.add(activationTemplate);
diff --git a/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/util/Annotates.java
b/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/util/Annotates.java
index 584076b..3911dd7 100644
--- a/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/util/Annotates.java
+++ b/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/util/Annotates.java
@@ -16,6 +16,7 @@ package org.apache.aries.cdi.container.internal.util;
 
 import static java.util.Arrays.asList;
 import static java.util.Optional.ofNullable;
+import static java.util.stream.Collectors.toList;
 import static org.apache.aries.cdi.container.internal.util.Reflection.getRawType;
 
 import java.lang.annotation.Annotation;
@@ -25,7 +26,6 @@ import java.lang.reflect.Type;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -58,6 +58,7 @@ import javax.inject.Qualifier;
 import javax.inject.Scope;
 import javax.interceptor.Interceptor;
 
+import org.apache.aries.cdi.container.internal.annotated.CachingAnnotated;
 import org.apache.aries.cdi.extension.spi.annotation.AdaptedService;
 import org.osgi.service.cdi.ServiceScope;
 import org.osgi.service.cdi.annotations.Service;
@@ -70,22 +71,32 @@ public class Annotates {
 	}
 
 	private static final Predicate<Annotation> isBeanDefining = annotation ->
-		ApplicationScoped.class.isAssignableFrom(annotation.annotationType()) ||
-		ConversationScoped.class.isAssignableFrom(annotation.annotationType()) ||
-		Decorator.class.isAssignableFrom(annotation.annotationType()) ||
-		Dependent.class.isAssignableFrom(annotation.annotationType()) ||
-		Interceptor.class.isAssignableFrom(annotation.annotationType()) ||
-		RequestScoped.class.isAssignableFrom(annotation.annotationType()) ||
-		SessionScoped.class.isAssignableFrom(annotation.annotationType()) ||
-		Stereotype.class.isAssignableFrom(annotation.annotationType());
+	{
+		// sun.reflect.annotation.AnnotationParser.annotationForMap is a proxy so locally cache
the type
+		final Class<? extends Annotation> annotationType = annotation.annotationType();
+		return ApplicationScoped.class.isAssignableFrom(annotationType) ||
+		ConversationScoped.class.isAssignableFrom(annotationType) ||
+		Decorator.class.isAssignableFrom(annotationType) ||
+		Dependent.class.isAssignableFrom(annotationType) ||
+		Interceptor.class.isAssignableFrom(annotationType) ||
+		RequestScoped.class.isAssignableFrom(annotationType) ||
+		SessionScoped.class.isAssignableFrom(annotationType) ||
+		Stereotype.class.isAssignableFrom(annotationType);
+	};
 
 	private static final Predicate<Annotation> isQualifier = annotation ->
-		!annotation.annotationType().equals(Qualifier.class) &&
-		annotation.annotationType().isAnnotationPresent(Qualifier.class);
+	{
+		final Class<? extends Annotation> annotationType = annotation.annotationType();
+		return !annotationType.equals(Qualifier.class) &&
+		annotationType.isAnnotationPresent(Qualifier.class);
+	};
 
 	private static final Predicate<Annotation> isScope = annotation ->
-		annotation.annotationType().isAnnotationPresent(Scope.class) ||
-		annotation.annotationType().isAnnotationPresent(NormalScope.class);
+	{
+		final Class<? extends Annotation> type = annotation.annotationType();
+		return type.isAnnotationPresent(Scope.class) ||
+		type.isAnnotationPresent(NormalScope.class);
+	};
 
 	public static Map<String, Object> componentProperties(Annotated annotated) {
 		return Maps.merge(annotated.getAnnotations());
@@ -157,17 +168,27 @@ public class Annotates {
 	}
 
 	public static Set<Annotation> collect(Annotated annotated, Predicate<Annotation>
predicate) {
-		return collect(annotated.getAnnotations()).stream().filter(predicate).collect(Collectors.toSet());
+		CachingAnnotated cachingAnnotated = CachingAnnotated.class.isInstance(annotated) ?
+				CachingAnnotated.class.cast(annotated) : null;
+		List<Annotation> cached = cachingAnnotated == null ? null : cachingAnnotated.getCollectedAnnotations();
+		if (cached != null) {
+			return cached.stream().filter(predicate).collect(Collectors.toSet());
+		}
+		List<Annotation> collected = collect(annotated.getAnnotations());
+		if (cachingAnnotated != null) {
+			cachingAnnotated.setCollectedAnnotations(collected);
+		}
+		return collected.stream().filter(predicate).collect(Collectors.toSet());
 	}
 
 	private static List<Annotation> collect(Collection<Annotation> annotations)
{
-		List<Annotation> list = new ArrayList<>();
-		for (Annotation a1 : annotations) {
-			if (a1.annotationType().getName().startsWith("java.lang.annotation.")) continue;
-			list.add(a1);
-		}
-		list.addAll(inherit(list));
-		return list;
+		return annotations.stream()
+				.flatMap(it -> Stream.concat(
+						Stream.of(it), Stream.of(it.annotationType().getAnnotations()))
+						.filter(a -> !a.annotationType().getName().startsWith("java.lang.annotation."))
+						.distinct())
+				.distinct()
+				.collect(toList());
 	}
 
 	private static List<Annotation> inherit(Collection<Annotation> annotations)
{
@@ -336,19 +357,21 @@ public class Annotates {
 	}
 
 	public static Class<? extends Annotation> beanScope(Annotated annotated, Class<?
extends Annotation> defaultValue) {
-		Class<? extends Annotation> scope = collect(annotated.getAnnotations()).stream().filter(isScope).map(Annotation::annotationType).findFirst().orElse(null);
-
-		return (scope == null) ? defaultValue : scope;
+		CachingAnnotated cachingAnnotated = CachingAnnotated.class.isInstance(annotated) ?
+				CachingAnnotated.class.cast(annotated) : null;
+		Class<? extends Annotation> cached = cachingAnnotated == null ? null : cachingAnnotated.getBeanScope();
+		if (cached == null) {
+			cached = collect(annotated.getAnnotations()).stream().filter(isScope).map(Annotation::annotationType).findFirst().orElse(null);
+			if (cachingAnnotated != null) {
+				cachingAnnotated.setBeanScope(cached);
+			}
+		}
+		return cached == null ? defaultValue : cached;
 	}
 
 	public static boolean hasBeanDefiningAnnotations(AnnotatedType<?> annotatedType) {
-		Set<Annotation> beanDefiningAnnotations = new HashSet<>();
-
-		beanDefiningAnnotations.addAll(collect(annotatedType, isBeanDefining));
-		beanDefiningAnnotations.addAll(annotatedType.getFields().stream().flatMap(field -> collect(field,
isBeanDefining).stream()).collect(Collectors.toSet()));
-		beanDefiningAnnotations.addAll(annotatedType.getMethods().stream().flatMap(method ->
collect(method, isBeanDefining).stream()).collect(Collectors.toSet()));
-
-		return !beanDefiningAnnotations.isEmpty();
+		// only on classes, not on producers
+		return !collect(annotatedType, isBeanDefining).isEmpty();
 	}
 
 }
\ No newline at end of file
diff --git a/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/util/Maps.java
b/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/util/Maps.java
index 5b457a0..41a5443 100644
--- a/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/util/Maps.java
+++ b/cdi-extender/src/main/java/org/apache/aries/cdi/container/internal/util/Maps.java
@@ -20,7 +20,6 @@ import static java.util.Objects.requireNonNull;
 import java.lang.annotation.Annotation;
 import java.util.AbstractMap;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Dictionary;
@@ -108,15 +107,6 @@ public class Maps {
 		).map(Map::entrySet).flatMap(Collection::stream));
 	}
 
-	public static Map<String, Object> merge(List<Dictionary<String, Object>>
dictionaries) {
-		return merge(dictionaries.stream().flatMap(Maps::streamOf));
-	}
-
-	@SafeVarargs
-	public static Map<String, Object> merge(Map<String, Object>... maps) {
-		return merge(Arrays.stream(maps).map(Map::entrySet).flatMap(Collection::stream));
-	}
-
 	public static Map<String, Object> merge(Stream<Map.Entry<String, Object>>
mapEntries) {
 		return mapEntries.collect(
 			Collectors.toMap(
@@ -127,15 +117,6 @@ public class Maps {
 		);
 	}
 
-	static Map<String, Object> addPrefix(Map<String, Object> map, String prefix)
{
-		return map.entrySet().stream().collect(
-			Collectors.toMap(
-				e -> prefix + e.getKey(),
-				Map.Entry::getValue
-			)
-		);
-	}
-
 	@SuppressWarnings("unchecked")
 	public static List<?> mergeValues(Object a, Object b) {
 		List<?> aList = Conversions.convert(a).to(new TypeReference<List<?>>()
{});
diff --git a/cdi-extender/src/test/java/org/apache/aries/cdi/container/internal/util/AnnotatesTest.java
b/cdi-extender/src/test/java/org/apache/aries/cdi/container/internal/util/AnnotatesTest.java
new file mode 100644
index 0000000..2e7494b
--- /dev/null
+++ b/cdi-extender/src/test/java/org/apache/aries/cdi/container/internal/util/AnnotatesTest.java
@@ -0,0 +1,39 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aries.cdi.container.internal.util;
+
+import org.apache.aries.cdi.container.internal.annotated.AnnotatedTypeImpl;
+import org.junit.Test;
+import org.osgi.service.cdi.annotations.Service;
+
+import javax.enterprise.context.ApplicationScoped;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class AnnotatesTest {
+    @Test
+    public void isBeanDefining() {
+        assertFalse(Annotates.hasBeanDefiningAnnotations(new AnnotatedTypeImpl<>(A.class)));
+        assertTrue(Annotates.hasBeanDefiningAnnotations(new AnnotatedTypeImpl<>(Single.class)));
+    }
+
+    @ApplicationScoped
+    public static class Single {
+    }
+
+    @Service
+    public static class A {
+    }
+}
diff --git a/cdi-itests/src/main/java/org/apache/aries/cdi/test/cases/TrimTests.java b/cdi-itests/src/main/java/org/apache/aries/cdi/test/cases/TrimTests.java
index d8b914d..9bf394d 100644
--- a/cdi-itests/src/main/java/org/apache/aries/cdi/test/cases/TrimTests.java
+++ b/cdi-itests/src/main/java/org/apache/aries/cdi/test/cases/TrimTests.java
@@ -25,13 +25,15 @@ import org.osgi.service.cdi.runtime.dto.ContainerDTO;
 public class TrimTests extends SlimBaseTestCase {
 
 	@Test
-	public void testTrimmed() throws Exception {
+	public void testTrimmed() {
 		Bundle tb2Bundle = installBundle.installBundle("tb17.jar");
 
 		ContainerDTO containerDTO = getContainerDTO(tb2Bundle);
 		assertNotNull(containerDTO);
 
-		assertEquals(5, containerDTO.template.components.get(0).beans.size());
+		assertEquals( // expected: B, E, F, G, H
+				String.join(", ", containerDTO.template.components.get(0).beans),
+				5, containerDTO.template.components.get(0).beans.size());
 
 		assertEquals(2, containerDTO.template.components.size());
 
diff --git a/cdi-itests/src/main/java/org/apache/aries/cdi/test/tb17/A.java b/cdi-itests/src/main/java/org/apache/aries/cdi/test/tb17/A.java
index acff571..0fbbf47 100644
--- a/cdi-itests/src/main/java/org/apache/aries/cdi/test/tb17/A.java
+++ b/cdi-itests/src/main/java/org/apache/aries/cdi/test/tb17/A.java
@@ -29,5 +29,4 @@ public class A implements Pojo {
 	public int getCount() {
 		return 1;
 	}
-
 }


Mime
View raw message