From commits-return-11503-archive-asf-public=cust-asf.ponee.io@aries.apache.org Fri Oct 30 10:20:51 2020 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mxout1-he-de.apache.org (mxout1-he-de.apache.org [95.216.194.37]) by mx-eu-01.ponee.io (Postfix) with ESMTPS id CEAA8180686 for ; Fri, 30 Oct 2020 11:20:51 +0100 (CET) Received: from mail.apache.org (mailroute1-lw-us.apache.org [207.244.88.153]) by mxout1-he-de.apache.org (ASF Mail Server at mxout1-he-de.apache.org) with SMTP id 05E3E62C41 for ; Fri, 30 Oct 2020 10:20:50 +0000 (UTC) Received: (qmail 82875 invoked by uid 500); 30 Oct 2020 10:20:50 -0000 Mailing-List: contact commits-help@aries.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@aries.apache.org Delivered-To: mailing list commits@aries.apache.org Received: (qmail 82852 invoked by uid 99); 30 Oct 2020 10:20:50 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 30 Oct 2020 10:20:50 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id C9EAF8040D; Fri, 30 Oct 2020 10:20:49 +0000 (UTC) Date: Fri, 30 Oct 2020 10:20:52 +0000 To: "commits@aries.apache.org" Subject: [aries-cdi] 03/03: [ARIES-2020] basic Discovery optimizations MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit From: rmannibucau@apache.org In-Reply-To: <160405324928.18532.9843703070256038772@gitbox.apache.org> References: <160405324928.18532.9843703070256038772@gitbox.apache.org> X-Git-Host: gitbox.apache.org X-Git-Repo: aries-cdi X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Rev: 2d071cbc932da248f56d937aa913890870c69d70 X-Git-NotificationType: diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated Message-Id: <20201030102049.C9EAF8040D@gitbox.apache.org> 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 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 implements Annotated { +public class AnnotatedImpl implements CachingAnnotated { private final Type _baseType; private final AnnotatedElement _annotatedElement; private final Set _typeClosure; + private Class beanScope; + private List collectedAnnotations; + public AnnotatedImpl(final Type baseType, final AnnotatedElement annotatedElement) { _baseType = baseType; _annotatedElement = annotatedElement; @@ -39,6 +43,26 @@ public class AnnotatedImpl implements Annotated { } @Override + public List getCollectedAnnotations() { + return collectedAnnotations; + } + + @Override + public void setCollectedAnnotations(final List collectedAnnotations) { + this.collectedAnnotations = collectedAnnotations; + } + + @Override + public Class getBeanScope() { + return beanScope; + } + + @Override + public void setBeanScope(Class 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 getBeanScope(); + void setBeanScope(Class value); + + // used in all "tests" so worth a cache + List getCollectedAnnotations(); + void setCollectedAnnotations(List 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 beanScope = Annotates.beanScope(annotatedType); - Map componentProperties = Annotates.componentProperties(annotatedType); - ServiceScope serviceScope = Annotates.serviceScope(annotatedType); - List 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 beanScope = Annotates.beanScope(annotatedField); - Map componentProperties = Annotates.componentProperties(annotatedField); - ServiceScope serviceScope = Annotates.serviceScope(annotatedField); List 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 beanScope = Annotates.beanScope(annotatedMethod); - Map componentProperties = Annotates.componentProperties(annotatedMethod); - ServiceScope serviceScope = Annotates.serviceScope(annotatedMethod); List 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 scope, List serviceTypeNames, ServiceScope serviceScope, Map componentProperties) { + void discoverActivations(OSGiBean osgiBean, Class declaringClass, final Annotated component, + AnnotatedMember producer, Class scope, + List 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 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 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 isQualifier = annotation -> - !annotation.annotationType().equals(Qualifier.class) && - annotation.annotationType().isAnnotationPresent(Qualifier.class); + { + final Class annotationType = annotation.annotationType(); + return !annotationType.equals(Qualifier.class) && + annotationType.isAnnotationPresent(Qualifier.class); + }; private static final Predicate isScope = annotation -> - annotation.annotationType().isAnnotationPresent(Scope.class) || - annotation.annotationType().isAnnotationPresent(NormalScope.class); + { + final Class type = annotation.annotationType(); + return type.isAnnotationPresent(Scope.class) || + type.isAnnotationPresent(NormalScope.class); + }; public static Map componentProperties(Annotated annotated) { return Maps.merge(annotated.getAnnotations()); @@ -157,17 +168,27 @@ public class Annotates { } public static Set collect(Annotated annotated, Predicate predicate) { - return collect(annotated.getAnnotations()).stream().filter(predicate).collect(Collectors.toSet()); + CachingAnnotated cachingAnnotated = CachingAnnotated.class.isInstance(annotated) ? + CachingAnnotated.class.cast(annotated) : null; + List cached = cachingAnnotated == null ? null : cachingAnnotated.getCollectedAnnotations(); + if (cached != null) { + return cached.stream().filter(predicate).collect(Collectors.toSet()); + } + List collected = collect(annotated.getAnnotations()); + if (cachingAnnotated != null) { + cachingAnnotated.setCollectedAnnotations(collected); + } + return collected.stream().filter(predicate).collect(Collectors.toSet()); } private static List collect(Collection annotations) { - List 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 inherit(Collection annotations) { @@ -336,19 +357,21 @@ public class Annotates { } public static Class beanScope(Annotated annotated, Class defaultValue) { - Class 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 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 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 merge(List> dictionaries) { - return merge(dictionaries.stream().flatMap(Maps::streamOf)); - } - - @SafeVarargs - public static Map merge(Map... maps) { - return merge(Arrays.stream(maps).map(Map::entrySet).flatMap(Collection::stream)); - } - public static Map merge(Stream> mapEntries) { return mapEntries.collect( Collectors.toMap( @@ -127,15 +117,6 @@ public class Maps { ); } - static Map addPrefix(Map 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>() {}); 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; } - }