[GR-14146] Preserve the JDK annotation implementation for run-time allocated annotations.

PullRequest: graal/3412

Author: cstancu

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/Inflation.java

@@ -75,6 +75,7 @@
 import com.oracle.svm.hosted.NativeImageClassLoader;
 import com.oracle.svm.hosted.SVMHost;
 import com.oracle.svm.hosted.analysis.flow.SVMMethodTypeFlowBuilder;
+import com.oracle.svm.hosted.meta.HostedType;
 import com.oracle.svm.hosted.substitute.AnnotationSubstitutionProcessor;
 
 import jdk.vm.ci.common.JVMCIError;
@@ -612,6 +613,16 @@ private static boolean isAnnotationUsed(AnalysisType annotationType) {
         return annotationInterfaceType.isInstantiated() || annotationInterfaceType.isInTypeCheck();
     }
 
+    public static ResolvedJavaType toWrappedType(ResolvedJavaType type) {
+        if (type instanceof AnalysisType) {
+            return ((AnalysisType) type).getWrappedWithoutResolve();
+        } else if (type instanceof HostedType) {
+            return ((HostedType) type).getWrapped().getWrappedWithoutResolve();
+        } else {
+            return type;
+        }
+    }
+
     @Override
     public boolean trackConcreteAnalysisObjects(AnalysisType type) {
         /*

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/annotation/AnnotationSubstitutionField.java

@@ -110,9 +110,7 @@ public JavaConstant readValue(JavaConstant receiver) {
                  * i.e., `jdk.proxy1`, cannot be open to all-unnamed-modules like we do with other
                  * modules.
                  */
-                Class<?>[] interfaces = proxy.getClass().getInterfaces();
-                assert interfaces.length == 1 : "Unexpected number of interfaces for annotation proxy class.";
-                Class<?> annotationInterface = interfaces[0];
+                Class<?> annotationInterface = AnnotationSupport.findAnnotationInterfaceTypeForMarkedAnnotationType(proxy.getClass());
                 Method reflectionMethod = annotationInterface.getDeclaredMethod(accessorMethod.getName());
                 reflectionMethod.setAccessible(true);
 

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/annotation/AnnotationSubstitutionType.java

@@ -26,21 +26,24 @@
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Proxy;
+import java.util.Arrays;
 
 import jdk.vm.ci.meta.MetaAccessProvider;
 import jdk.vm.ci.meta.ResolvedJavaType;
 
 public class AnnotationSubstitutionType extends CustomSubstitutionType<AnnotationSubstitutionField, AnnotationSubstitutionMethod> {
 
     private final String name;
+    private final MetaAccessProvider metaAccess;
 
     public AnnotationSubstitutionType(MetaAccessProvider metaAccess, ResolvedJavaType original) {
         super(original);
+        this.metaAccess = metaAccess;
 
         assert original.getSuperclass().equals(metaAccess.lookupJavaType(Proxy.class));
         assert metaAccess.lookupJavaType(Annotation.class).isAssignableFrom(original);
 
-        ResolvedJavaType annotationInterfaceType = AnnotationSupport.findAnnotationInterfaceType(original);
+        ResolvedJavaType annotationInterfaceType = AnnotationSupport.findAnnotationInterfaceTypeForMarkedAnnotationType(original, metaAccess);
         assert annotationInterfaceType.isAssignableFrom(original);
         assert metaAccess.lookupJavaType(Annotation.class).isAssignableFrom(annotationInterfaceType);
 
@@ -49,6 +52,17 @@ public AnnotationSubstitutionType(MetaAccessProvider metaAccess, ResolvedJavaTyp
         name = n.substring(0, n.length() - 1) + "$$ProxyImpl;";
     }
 
+    @Override
+    public ResolvedJavaType[] getInterfaces() {
+        /* Filter out the ConstantAnnotationMarker interface. */
+        ResolvedJavaType[] interfaces = super.getInterfaces();
+        return Arrays.stream(interfaces).filter(this::isNotAnnotationMarkerInterface).toArray(ResolvedJavaType[]::new);
+    }
+
+    private boolean isNotAnnotationMarkerInterface(ResolvedJavaType type) {
+        return !type.equals(metaAccess.lookupJavaType(ConstantAnnotationMarker.class));
+    }
+
     @Override
     public String getName() {
         return name;

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/annotation/AnnotationSupport.java

@@ -26,7 +26,10 @@
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Modifier;
+import java.lang.reflect.Proxy;
 import java.util.Arrays;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
 
 import org.graalvm.compiler.api.replacements.SnippetReflectionProvider;
 import org.graalvm.compiler.core.common.type.StampFactory;
@@ -56,14 +59,19 @@
 import org.graalvm.compiler.replacements.GraphKit;
 import org.graalvm.compiler.replacements.nodes.BasicObjectCloneNode;
 import org.graalvm.nativeimage.ImageSingletons;
+import org.graalvm.nativeimage.hosted.Feature;
 
 import com.oracle.graal.pointsto.constraints.UnsupportedFeatureException;
 import com.oracle.graal.pointsto.meta.HostedProviders;
+import com.oracle.svm.core.annotate.AutomaticFeature;
 import com.oracle.svm.core.annotate.Substitute;
 import com.oracle.svm.core.annotate.TargetClass;
 import com.oracle.svm.core.hub.AnnotationTypeSupport;
 import com.oracle.svm.core.jdk.AnnotationSupportConfig;
 import com.oracle.svm.core.meta.SubstrateObjectConstant;
+import com.oracle.svm.core.util.VMError;
+import com.oracle.svm.hosted.FeatureImpl.DuringSetupAccessImpl;
+import com.oracle.svm.hosted.analysis.Inflation;
 import com.oracle.svm.hosted.phases.HostedGraphKit;
 import com.oracle.svm.hosted.snippets.SubstrateGraphBuilderPlugins;
 
@@ -85,24 +93,35 @@ public class AnnotationSupport extends CustomSubstitution<AnnotationSubstitution
 
     private final ResolvedJavaType javaLangAnnotationAnnotation;
     private final ResolvedJavaType javaLangReflectProxy;
+    private final ResolvedJavaType constantAnnotationMarker;
 
     public AnnotationSupport(MetaAccessProvider metaAccess, SnippetReflectionProvider snippetReflection) {
         super(metaAccess);
         this.snippetReflection = snippetReflection;
 
         javaLangAnnotationAnnotation = metaAccess.lookupJavaType(java.lang.annotation.Annotation.class);
         javaLangReflectProxy = metaAccess.lookupJavaType(java.lang.reflect.Proxy.class);
+        constantAnnotationMarker = metaAccess.lookupJavaType(ConstantAnnotationMarker.class);
 
         AnnotationSupportConfig.initialize();
     }
 
-    private boolean isAnnotation(ResolvedJavaType type) {
-        return javaLangAnnotationAnnotation.isAssignableFrom(type) && javaLangReflectProxy.isAssignableFrom(type);
+    private boolean isConstantAnnotationType(ResolvedJavaType type) {
+        /*
+         * Check if the type implements all of Annotation, Proxy and ConstantAnnotationMarker. If
+         * so, then it is the type of a annotation proxy object encountered during heap scanning.
+         * Only those types are substituted with a more efficient annotation proxy type
+         * implementation. If a type implements only Annotation and Proxy but not
+         * ConstantAnnotationMarker then it is a proxy type registered via the dynamic proxy API.
+         * Such type is used to allocate annotation instances at run time and must not be replaced.
+         */
+        return javaLangAnnotationAnnotation.isAssignableFrom(type) && javaLangReflectProxy.isAssignableFrom(type) &&
+                        constantAnnotationMarker.isAssignableFrom(type);
     }
 
     @Override
     public ResolvedJavaType lookup(ResolvedJavaType type) {
-        if (isAnnotation(type)) {
+        if (isConstantAnnotationType(type)) {
             return getSubstitution(type);
         }
         return type;
@@ -118,15 +137,15 @@ public ResolvedJavaType resolve(ResolvedJavaType type) {
 
     @Override
     public ResolvedJavaField lookup(ResolvedJavaField field) {
-        if (isAnnotation(field.getDeclaringClass())) {
+        if (isConstantAnnotationType(field.getDeclaringClass())) {
             throw new UnsupportedFeatureException("Field of annotation proxy is not accessible: " + field);
         }
         return field;
     }
 
     @Override
     public ResolvedJavaMethod lookup(ResolvedJavaMethod method) {
-        if (isAnnotation(method.getDeclaringClass())) {
+        if (isConstantAnnotationType(method.getDeclaringClass())) {
             AnnotationSubstitutionType declaringClass = getSubstitution(method.getDeclaringClass());
             AnnotationSubstitutionMethod result = declaringClass.getSubstitutionMethod(method);
             assert result != null && result.original.equals(method);
@@ -286,8 +305,8 @@ static class AnnotationAnnotationTypeMethod extends AnnotationSubstitutionMethod
 
         @Override
         public StructuredGraph buildGraph(DebugContext debug, ResolvedJavaMethod method, HostedProviders providers, Purpose purpose) {
-            assert method.getDeclaringClass().getInterfaces().length == 1;
-            ResolvedJavaType annotationInterfaceType = method.getDeclaringClass().getInterfaces()[0];
+            ResolvedJavaType annotationType = method.getDeclaringClass();
+            ResolvedJavaType annotationInterfaceType = findAnnotationInterfaceType(annotationType);
             JavaConstant returnValue = providers.getConstantReflection().asJavaClass(annotationInterfaceType);
 
             GraphKit kit = new HostedGraphKit(debug, providers, method);
@@ -473,11 +492,42 @@ public StructuredGraph buildGraph(DebugContext debug, ResolvedJavaMethod method,
         }
     }
 
-    static ResolvedJavaType findAnnotationInterfaceType(ResolvedJavaType annotationType) {
-        assert annotationType.getInterfaces().length == 1;
-        return annotationType.getInterfaces()[0];
+    /*
+     * This method retrieves the annotation interface type from an annotation proxy type represented
+     * as an AnnotationSubstitutionType (or a type that wraps an AnnotationSubstitutionType). The
+     * ConstantAnnotationMarker interface is already filtered out when
+     * AnnotationSubstitutionType.getInterfaces() is called.
+     */
+    private static ResolvedJavaType findAnnotationInterfaceType(ResolvedJavaType annotationType) {
+        VMError.guarantee(Inflation.toWrappedType(annotationType) instanceof AnnotationSubstitutionType);
+        ResolvedJavaType[] interfaces = annotationType.getInterfaces();
+        VMError.guarantee(interfaces.length == 1, "Unexpected number of interfaces for annotation proxy class.");
+        return interfaces[0];
     }
 
+    /**
+     * This method retrieves the annotation interface type from a marked annotation proxy type.
+     * Annotation proxy types implement only the annotation interface by default. However, since we
+     * inject the ConstantAnnotationMarker the Annotation proxy types for ahead-of-time allocated
+     * annotations implement two interfaces. We make sure we return the right one here.
+     */
+    static ResolvedJavaType findAnnotationInterfaceTypeForMarkedAnnotationType(ResolvedJavaType annotationType, MetaAccessProvider metaAccess) {
+        ResolvedJavaType[] interfaces = annotationType.getInterfaces();
+        VMError.guarantee(interfaces.length == 2, "Unexpected number of interfaces for annotation proxy class.");
+        VMError.guarantee(interfaces[1].equals(metaAccess.lookupJavaType(ConstantAnnotationMarker.class)));
+        return interfaces[0];
+    }
+
+    /*
+     * This method is similar to the above one, with the difference that it takes a Class<?> instead
+     * of an ResolvedJavaType as an argument.
+     */
+    static Class<?> findAnnotationInterfaceTypeForMarkedAnnotationType(Class<? extends Proxy> clazz) {
+        Class<?>[] interfaces = clazz.getInterfaces();
+        VMError.guarantee(interfaces.length == 2, "Unexpected number of interfaces for annotation proxy class.");
+        VMError.guarantee(interfaces[1].equals(ConstantAnnotationMarker.class));
+        return interfaces[0];
+    }
 }
 
 @TargetClass(className = "sun.reflect.annotation.AnnotationType")
@@ -498,3 +548,63 @@ public static AnnotationType getInstance(Class<? extends Annotation> annotationC
     }
 
 }
+
+@AutomaticFeature
+class AnnotationSupportFeature implements Feature {
+
+    @Override
+    public void duringSetup(DuringSetupAccess access) {
+        DuringSetupAccessImpl config = (DuringSetupAccessImpl) access;
+        access.registerObjectReplacer(new AnnotationObjectReplacer(config.getImageClassLoader().getClassLoader()));
+    }
+}
+
+/**
+ * This replacer replaces the annotation proxy instances with a clone that additionaly implements
+ * the ConstantAnnotationMarker interface.
+ */
+class AnnotationObjectReplacer implements Function<Object, Object> {
+
+    private final ClassLoader classLoader;
+    /**
+     * Cache the replaced objects to ensure that they are only replaced once. We are using a
+     * concurrent hash map because replace() may be called from BigBang.finish(), which is
+     * multi-threaded.
+     * 
+     * A side effect of this caching is de-duplication of annotation instances. When running as a
+     * native image two equal annotation instances are also identical. On HotSpot that is not true,
+     * the two annotation instances, although equal, are actually two distinct objects. Although
+     * this is a small deviation from HotSpot semantics it can improve the native image size.
+     * 
+     * If de-duplication is not desired that can be achieved by replacing the ConcurrentHashMap with
+     * an IdentityHashMap (and additional access synchronisation).
+     */
+    private ConcurrentHashMap<Object, Object> objectCache = new ConcurrentHashMap<>();
+
+    AnnotationObjectReplacer(ClassLoader loader) {
+        this.classLoader = loader;
+    }
+
+    @Override
+    public Object apply(Object original) {
+        Class<?> clazz = original.getClass();
+        if (Annotation.class.isAssignableFrom(clazz) && Proxy.class.isAssignableFrom(clazz)) {
+            return objectCache.computeIfAbsent(original, obj -> replacementComputer(obj, classLoader));
+        }
+
+        return original;
+    }
+
+    /**
+     * Effectively clones the original proxy object and it adds the ConstantAnnotationMarker
+     * interface.
+     */
+    private static Object replacementComputer(Object original, ClassLoader classLoader) {
+        Class<?>[] interfaces = original.getClass().getInterfaces();
+        Class<?>[] extendedInterfaces = Arrays.copyOf(interfaces, interfaces.length + 1);
+        extendedInterfaces[extendedInterfaces.length - 1] = ConstantAnnotationMarker.class;
+
+        return Proxy.newProxyInstance(classLoader, extendedInterfaces, Proxy.getInvocationHandler(original));
+    }
+
+}

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/annotation/ConstantAnnotationMarker.java

@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) 2019, 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package com.oracle.svm.hosted.annotation;
+
+import org.graalvm.nativeimage.Platform;
+import org.graalvm.nativeimage.Platforms;
+
+/**
+ * This interface is used to mark the ahead-of-time allocated annotation proxy objects. These
+ * objects have an optimized type which removes the overhead of storing the annotation values in a
+ * HashMap. See AnnotationSupport#getSubstitution(ResolvedJavaType) for the logic where this
+ * substitution is implemented. This is possible since the ahead-of-time allocated annotation proxy
+ * objects are effectively constant.
+ * 
+ * The {@link ConstantAnnotationMarker} is removed before runtime. See
+ * {@link AnnotationSubstitutionType#getInterfaces()}. This is also enforced through the HOSTED_ONLY
+ * annotation below. No {@link ConstantAnnotationMarker} is allowed in the image heap.
+ * 
+ * The run-time allocated annotations use the default JDK implementation.
+ * 
+ * The downside of having a separate (more efficient) implementation for ahead-of-time allocated
+ * annotations is that at run time there can be two proxy types for the same annotation type and an
+ * equality check between them would fail.
+ */
+@Platforms(Platform.HOSTED_ONLY.class)
+public interface ConstantAnnotationMarker {
+}

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/ClassInitializationFeature.java

@@ -51,7 +51,7 @@
 import com.oracle.svm.core.util.UserError;
 import com.oracle.svm.hosted.FeatureImpl;
 import com.oracle.svm.hosted.NativeImageOptions;
-import com.oracle.svm.hosted.meta.HostedType;
+import com.oracle.svm.hosted.analysis.Inflation;
 import com.oracle.svm.hosted.meta.MethodPointer;
 import com.oracle.svm.hosted.phases.SubstrateClassInitializationPlugin;
 
@@ -281,23 +281,12 @@ static boolean declaresDefaultMethods(ResolvedJavaType type) {
          * We call getDeclaredMethods() directly on the wrapped type. We avoid calling it on the
          * AnalysisType because it resolves all the methods in the AnalysisUniverse.
          */
-        for (ResolvedJavaMethod method : toWrappedType(type).getDeclaredMethods()) {
+        for (ResolvedJavaMethod method : Inflation.toWrappedType(type).getDeclaredMethods()) {
             if (method.isDefault()) {
                 assert !Modifier.isStatic(method.getModifiers()) : "Default method that is static?";
                 return true;
             }
         }
         return false;
     }
-
-    private static ResolvedJavaType toWrappedType(ResolvedJavaType type) {
-        if (type instanceof AnalysisType) {
-            return ((AnalysisType) type).getWrappedWithoutResolve();
-        } else if (type instanceof HostedType) {
-            return ((HostedType) type).getWrapped().getWrappedWithoutResolve();
-        } else {
-            return type;
-        }
-    }
-
 }