Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not load java.awt.Component and java.beans.Customizer classes when they are not used in Java Beans #5065

Conversation

AlexanderScherbatiy
Copy link
Contributor

GraalVM includes java.awt.Component and derived classes into native image even a Java Beans application does not use them.

Steps to reproduce:
Compile the BeanSample.java class which does not define a bean customizer, create a reflection configuration file by the native-image-agent, generate a native image with the config file and --diagnostics-mode option to check classes included into the native image:

  > javac BeanSample.java
  > java -agentlib:native-image-agent=config-output-dir=conf-dir BeanSample
  > native-image --no-fallback --diagnostics-mode -H:ReflectionConfigurationFiles=conf-dir/reflect-config.json BeanSample

The class_initialization_report_date.csv file reports some unused java.awt classes: java.awt.Component, java.awt.Container, ava.awt.EventQueue, and others.

Suggested solution:
Avoid direct java.awt.Component class usage in Introspector.findCustomizerClass() method. Change explicit Component.class.isAssignableFrom(type) method call to reflection Class.forName("java.awt.Component").isAssignableFrom(type).

The fix proposes Introspector.findCustomizerClass() substitution method:

  • Component.class.isAssignableFrom(type) and Customizer.class.isAssignableFrom(type) calls are are substituted by their reflection counterparts Class.forName(COMPONENT_CLASS).isAssignableFrom(type) and Class.forName(CUSTOMIZER_CLASS).isAssignableFrom(type)
  • COMPONENT_CLASS and CUSTOMIZER_CLASS fields are made private static but not final to avoid class name interception by GraalVM.

With the fix the class_initialization_report_date.csv does not contain java.awt.Component and derived classes for the BeanSample.

The BeanSampleCustomizer sample shows that the fix works as well for the case when Java Beans Customizer is used and necesary java.awt.Component classes are included into the native image:

> native-image --no-fallback --diagnostics-mode -H:ReflectionConfigurationFiles=conf-dir/reflect-config.json BeanSampleCustomizer

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. label Sep 23, 2022
@oracle-contributor-agreement
Copy link

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
The following contributors of this PR have not signed the OCA:

To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application.

When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated.

@AlexanderScherbatiy
Copy link
Contributor Author

@sdeleuze

@sdeleuze
Copy link
Collaborator

sdeleuze commented Oct 3, 2022

@AlexanderScherbatiy Thanks!

@vjovanov This is the fix to the Awt component footpring issue we discussed with Bellsoft since a fix on OpenJdk side was unlikely to be merged.

@sdeleuze sdeleuze added the spring spring related issue label Oct 3, 2022
@AlexanderScherbatiy
Copy link
Contributor Author

I have sent adding to the list of contributors for my company agreement request to the opensource_ww_grp@oracle.com alias.

@vjovanov vjovanov self-requested a review October 3, 2022 17:50
@vjovanov vjovanov added this to To do in Native Image via automation Oct 3, 2022
@vjovanov
Copy link
Member

vjovanov commented Oct 4, 2022

@vjovanov This is the fix to the Awt component footprint issue we discussed with Bellsoft since a fix on OpenJdk side was unlikely to be merged.

I would suggest opening a ticket on the JDK issue tracker for this and submitting a PR. Then we can merge this as we did everything we can.

@TargetClass(className = "java.beans.Introspector")
static final class Target_java_beans_Introspector {

// Do not load java.awt.Component and java.beans.Customizer classes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a doc comment.

@AlexanderScherbatiy
Copy link
Contributor Author

I would suggest opening a ticket on the JDK issue tracker for this and submitting a PR. Then we can merge this as we did everything we can.

It looks like the fix is graal specific and is not useful to JDK itself because java.awt.Component class is not loaded if the Component.class.isAssignableFrom(type) code path is not reached by the program.

For example the BeanSample class below does not define Java Beans customizer. The method Introspector.findCustomizerClass() is called but it returns before the Component.class.isAssignableFrom(type) check so java.awt.Component and derived classes are not loaded.

java -verbose:class BeanSample command does not log java.awt.* classes in this case.

import java.beans.Introspector;

public class BeanSample {

    public static class BeanClass {
    }

    public static void main(String[] args) throws Exception {
        System.out.printf("load customizer for class: %s%n", BeanClass.class);
        Class<?> cls = Introspector.getBeanInfo(BeanClass.class).getBeanDescriptor().getCustomizerClass();
        System.out.printf("customizer: %s%n", cls);
    }
}

It seems that there is no benefit for JDK tools like jpackage and jlink as well because they add the whole java.desktop module as a dependency.

The graal fix moves the Component class name from Class.forName("java.awt.Component") to private static variable to avoid class name interception. It also relies on specific graal behavior which is not applicable to JDK.

String name = type.getName() + "Customizer";
try {
type = ClassFinder.findClass(name, type.getClassLoader());
if (Class.forName(COMPONENT_CLASS).isAssignableFrom(type)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot use Class.forName in this context because that would alter the behavior significantly: it would require a manual reflection configuration of those two classes, which no application would do properly and therefore code that actually relies on this method would no longer work properly.

The proper solution is a reachability handler: when java.awt.Component is reachable for any other reason, then a check for it is needed here too - but this code should not make it reachable itself. So we need a pattern similar to ProtectionDomainSupport/ProtectionDomainFeature: in a reachability handler for java.awt.Component, a non-final field Class<?> COMPONENT_CLASS is set to java.awt.Component.class. As long as that field is null, no isAssignableFrom check is done (and no check is necessary because that type is unreachable anyways).

But of course you found a propbably larger pattern where we currently include too much in the image: the problem is actually not that Component gets reachable - because a type being reachable has very little impact on image size - but that the static initializer of Component gets reachable. Currently we treat these two conditions as the same, but there is no good reason for that. We only need to make the static initializer reachable if a class could be initialized, and an instanceof check never triggers class initialization. I will think more about this and see if we can come up with a general solution that makes a substitution like this unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is the question which is not clear for me. The code in question is:

    private static Class<?> findCustomizerClass(Class<?> type) {
        String name = type.getName() + "Customizer";
        try {
            type = ClassFinder.findClass(name, type.getClassLoader());
            // Each customizer should inherit java.awt.Component and implement java.beans.Customizer
            // according to the section 9.3 of JavaBeans specification
            if (Component.class.isAssignableFrom(type) && Customizer.class.isAssignableFrom(type)) {
                ...

Is it possible for graal to understand that if a class Foo is passed to findCustomizerClass(...) method then the concatenation the Foo class name with "Customizer" string leads to a class which can't be found by ClassFinder.findClass(...) method (which is actually Class.forName() call with the given class loader)?

Or could you give more details about a reachability handler which can help in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot use Class.forName in this context because that would alter the behavior significantly: it would require a manual reflection configuration of those two classes, which no application would do properly and therefore code that actually relies on this method would no longer work properly.

For the code in question I see three use cases:

  1. Customizer class is not defined for the bean class
  2. Customizer class is properly defined (extends Component class and implements Customizer interface) for the bean class
  3. Customizer class is not properly defined (does not extend Component class or do not implement Customizer interface)

For the first case the application does not use the Component class at all and the behavior of running a program by java or native image gives the same result.

For the second case I used BeanSample example:

import java.beans.Introspector;

public class BeanSample {

    public static class BeanClass {
    }

    public static class BeanClassCustomizer extends java.awt.Component implements java.beans.Customizer {
        public void setObject(Object bean) {
            throw new UnsupportedOperationException();
        }
    }

    public static void main(String[] args) throws Exception {
        System.out.printf("load customizer for class: %s%n", BeanClass.class);
        Class<?> cls = Introspector.getBeanInfo(BeanClass.class).getBeanDescriptor().getCustomizerClass();
        System.out.printf("customizer: %s%n", cls);

    }
}

Java returns BeanSample$BeanClassCustomizer.

> java BeanSample
load customizer for class: class BeanSample$BeanClass
customizer: class BeanSample$BeanClassCustomizer

A native image which has been made without an additional configuration returns null customizer:

> native-image  --no-fallback BeanSample
> ./beansample
load customizer for class: class BeanSample$BeanClass
customizer: null

This is because the BeanClassCustomizer is only created by reflection.
It works if the reflect-config.json file is defined:

[
{
  "name":"BeanSample$BeanClass",
  "queryAllPublicMethods":true
},
{
  "name":"BeanSample$BeanClassCustomizer"
},
{
  "name":"java.beans.PropertyVetoException"
},
{
  "name":"java.lang.Object",
  "queryAllPublicMethods":true
}
]

Now the BeanSample$BeanClassCustomizer is returned:

> native-image --no-fallback -H:ReflectionConfigurationFiles=conf-dir/reflect-config.json BeanSample
> ./beansample
load customizer for class: class BeanSample$BeanClass
customizer: class BeanSample$BeanClassCustomizer

For the third case where the bean customizer does not extend Component class I used the BeanSample:

import java.beans.Introspector;

public class BeanSample {

    public static class BeanClass {
    }


    public static class BeanClassCustomizer {
    }

    public static void main(String[] args) throws Exception {
        System.out.printf("load customizer for class: %s%n", BeanClass.class);
        Class<?> cls = Introspector.getBeanInfo(BeanClass.class).getBeanDescriptor().getCustomizerClass();
        System.out.printf("customizer: %s%n", cls);

    }
}

Java returns null bean customizer:

> java BeanSample
load customizer for class: class BeanSample$BeanClass
customizer: null

The native image with reflection config file returns null customizer as well:

native-image --no-fallback --diagnostics-mode -H:ReflectionConfigurationFiles=conf-dir/reflect-config.json BeanSample
> ./beansample
load customizer for class: class BeanSample$BeanClass
customizer: null

As I see, for all three cases the result which is returned by Introspector.findCustomizerClass() method is the same.

For the fist case the Component class is not used at all.
For the second case the Component class is used by a beans customizer in a user program and can be derived by graal.
For the third case where a customizer does not extend the Component class the customizer class is handled in different ways in java ( the Component.class.isAssignableFrom(type) check fails) and in native image (ClassNotFoundException: java.awt.Component is thrown) but the result is the same because all exception are just ignored by Introspector.findCustomizerClass() method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that in the next release we plan to throw special runtime exceptions for missing metadata, so the try/catch block will not catch this:

#5171

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix is updated to use the reachability handler for java.awt.Component class.

Two classes JavaBeansSupport/JavaBeansFeature are added in the similar way as ProtectionDomainSupport/ProtectionDomainFeature.

COMPONENT_CLASS is only set to java.awt.Component if it is reachable in the program.
Introspector.findCustomizerClass() substitution method returns null when COMPONENT_CLASS is not set because the customizer does not extend java.awt.Component in this case.

Native Image automation moved this from To do to In progress Oct 4, 2022
import com.sun.beans.finder.ClassFinder;

@SuppressWarnings({"static-method", "unused"})
public final class JavaBeansSupport {
Copy link
Member

@vjovanov vjovanov Oct 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@christianwimmer do we have a ticket for the analysis imprecision related to <clinit>? I think we should link it here so we delete this once the analysis imprecision is fixed.

@christianwimmer
Copy link
Member

After a few false starts into the wrong direction, I now have a PR to fix the problem in the static analysis itself: #5224

It turns out that it is not even necessary to decouple the reachability of a type from the reachability of the type's clinit method. Instead, a type check just does not need to mark the type as reachable in the first place.

@sdeleuze
Copy link
Collaborator

@AlexanderScherbatiy I think this PR can be closed unmerged since #5224 fixes the root issue.

@AlexanderScherbatiy
Copy link
Contributor Author

Fixed as part of #5224

Native Image automation moved this from In progress to Done Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. spring spring related issue
Projects
Native Image
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants