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

Static methods do not allow code removal while static fields do #2594

Closed
sdeleuze opened this issue Jun 20, 2020 · 7 comments
Closed

Static methods do not allow code removal while static fields do #2594

sdeleuze opened this issue Jun 20, 2020 · 7 comments
Assignees
Labels
bug native-image spring spring related issue
Milestone

Comments

@sdeleuze
Copy link
Collaborator

Describe the issue
Static boolean fields of classes initialized at build time are effective for code removal when used in if statements while static methods returning the very same field are not.

Steps to reproduce the issue
Consider this simple class (initialized at build time) used to detect if the code is running as a native image or not:

public abstract class NativeImageDetector {

	public static final boolean IN_NATIVE_IMAGE = (System.getProperty("org.graalvm.nativeimage.imagecode") != null);

	public static boolean inNativeImage() {
		return IN_NATIVE_IMAGE;
	}
}

If the static field is used in an if statement:

public class AppWithStaticField {

    public static void main(String[] args) {
        if (!NativeImageDetector.IN_NATIVE_IMAGE) {
            System.out.println("Should not be printed in native images" + new Foo().foo());
        }
    }
}

As expected the message is not printed on the console and com.sample.Foo is not referenced in the used classes visible in the report generated via -H:+PrintAnalysisCallTree.

But if we do the same with the static method:

public class AppWithStaticMethod {

    public static void main(String[] args) {
        if (!NativeImageDetector.inNativeImage()) {
            System.out.println("Should not be printed in native images" + new Foo().foo());
        }
    }
}

As expected the message is not printed on the console.
But com.sample.Foo is referenced in the used classes visible in the report generated via `-H:+PrintAnalysisCallTree. Such class should be not be referenced here nor part of the static image, the static method and static field variants should be equivalent for such trivial level of indirection.

Check https://github.com/sdeleuze/graalvm-native-issues/tree/master/static-method-code-removal-regression and run ./build.sh to reproduce the issue.

Describe GraalVM and your environment:

  • Tested with GraalVM 20.1.0 and 20.0.0 (Java 8 variant)
sdeleuze added a commit to sdeleuze/spring-framework that referenced this issue Jun 20, 2020
GraalDetector has been removed for now because of
oracle/graal#2594. It should be reintroduced
when this bug will be fixed with NativeImageDetector class name.

Closes spring-projectsgh-25179
sdeleuze added a commit to sdeleuze/spring-framework that referenced this issue Jun 20, 2020
This commit removes load time weaving, CGLIB and Objenesis support
from native images.

GraalDetector has been removed for now because of
oracle/graal#2594. It should be reintroduced
when this bug will be fixed with NativeImageDetector class name.

Closes spring-projectsgh-25179
sdeleuze added a commit to spring-projects/spring-framework that referenced this issue Jun 20, 2020
This commit removes load time weaving, CGLIB and Objenesis support
from native images.

GraalDetector has been removed for now because of
oracle/graal#2594. It should be reintroduced
when this bug will be fixed with NativeImageDetector class name.

Closes gh-25179
kenny5he pushed a commit to kenny5he/spring-framework that referenced this issue Jun 21, 2020
This commit removes load time weaving, CGLIB and Objenesis support
from native images.

GraalDetector has been removed for now because of
oracle/graal#2594. It should be reintroduced
when this bug will be fixed with NativeImageDetector class name.

Closes spring-projectsgh-25179
@eginez eginez added this to To do in Native Image via automation Jun 22, 2020
@eginez eginez added the spring spring related issue label Jun 22, 2020
@christianwimmer
Copy link
Member

The static analysis currently is not context sensitive and also does not do any inlining. Therefore, even tiny helper methods like inNativeImage are not inlined and constant folded.

We have a research project that tries to address this limitation. However, the solution is not as trivial as it sounds ("just always inline this method"), because we really need to guarantee that inlining happens in all places, which requires a bit of implementation.

@eginez eginez closed this as completed Jun 22, 2020
Native Image automation moved this from To do to Done Jun 22, 2020
@sdeleuze
Copy link
Collaborator Author

sdeleuze commented Jul 3, 2020

@eginez I am not sure to understand why this issue is closed, despite being non trivial to fix isn't it a real open issue?

See for example https://tomcat.apache.org/tomcat-9.0-doc/api/org/apache/tomcat/util/compat/JreCompat.html#isGraalAvailable-- that shows that Tomcat team and likely many others have the same expectation than us.

Is there a root issue to track progresses on you research project?

@eginez
Copy link
Contributor

eginez commented Jul 8, 2020

@sdeleuze I think it makes sense to keep this open to track future development on this issue, for now, however nothing is planned

@eginez eginez reopened this Jul 8, 2020
Native Image automation moved this from Done to In progress Jul 8, 2020
@eginez eginez moved this from In progress to To do in Native Image Jul 9, 2020
fhanik added a commit to fhanik/tomcat that referenced this issue Aug 7, 2020
Testing if Apache Tomcat is affected
@sdeleuze
Copy link
Collaborator Author

@vjovanov @christianwimmer As discussed recently, could we move this issue to 20.3 since it seems the potential fix for #2500 will fix this one as well?

@marjanasolajic
Copy link
Contributor

@vjovanov @christianwimmer As discussed recently, could we move this issue to 20.3 since it seems the potential fix for #2500 will fix this one as well?

We have a research project that looks at method inlining before static analysis #2860. In #2860 (comment) you'll find examples for fixing #2500.

@vjovanov
Copy link
Member

vjovanov commented Dec 15, 2020

This should be addressed by the following commit: 17f1de4

Feel free to try this out by using -H:+InlineBeforeAnalysis.

@sdeleuze
Copy link
Collaborator Author

sdeleuze commented Jan 5, 2021

I confirm -H:+InlineBeforeAnalysis works as expected for https://github.com/sdeleuze/graalvm-native-issues/tree/master/static-method-code-removal-regression use case and will allow Spring and Tomcat utility method usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug native-image spring spring related issue
Projects
Native Image
  
Done
Development

No branches or pull requests

6 participants