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

Native Image #330

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Native Image #330

wants to merge 3 commits into from

Conversation

lorenzleutgeb
Copy link
Member

@lorenzleutgeb lorenzleutgeb commented Dec 20, 2021

Add support for compiling Alpha to ELF x86_64 executables.

https://github.com/alpha-asp/Alpha/runs/4579874459?check_suite_focus=true#step:8:1

@codecov
Copy link

codecov bot commented Dec 20, 2021

Codecov Report

Merging #330 (6366e79) into master (d42818a) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #330   +/-   ##
=========================================
  Coverage     70.23%   70.23%           
  Complexity     2123     2123           
=========================================
  Files           182      182           
  Lines          8023     8023           
  Branches       1423     1423           
=========================================
  Hits           5635     5635           
  Misses         2028     2028           
  Partials        360      360           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d42818a...6366e79. Read the comment docs.

Copy link
Collaborator

@madmike200590 madmike200590 left a comment

Choose a reason for hiding this comment

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

As discussed, there's currently a problem with resources being included.
The stringtemplate resources are missing from the created image, therefore all attempts to solve programs end with a NullPointerException.

@AntoniusW
Copy link
Collaborator

Is that really what I think it is, do we have a native binary of Alpha now? :-D

@lorenzleutgeb
Copy link
Member Author

lorenzleutgeb commented Dec 20, 2021

As discussed, there's currently a problem with resources being included. The stringtemplate resources are missing from the created image, therefore all attempts to solve programs end with a NullPointerException.

Should be resolved now, see https://github.com/alpha-asp/Alpha/runs/4585139250?check_suite_focus=true#step:9:14

Is that really what I think it is, do we have a native binary of Alpha now? :-D

Yep. But even merging this PR, it will need further testing, and quite possibly a few more tweaks (especially regarding Reflection and Resources). So consider it an "experimental feature". We're getting there.

Copy link
Collaborator

@madmike200590 madmike200590 left a comment

Choose a reason for hiding this comment

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

Unfortunately we still have some problems related to classpath resources, see:

...$ ./alpha-native-20220110 -str "len(X) :- &stdlib_string_length["bla"](X)."
749 WARN org.reflections.Reflections - given scan urls are empty. set urls in the configuration
Exception in thread "main" java.lang.IllegalArgumentException: Unknown interpretation name encountered: stdlib_string_length
	at at.ac.tuwien.kr.alpha.core.parser.ParseTreeVisitor.visitExternal_atom(ParseTreeVisitor.java:553)
	at at.ac.tuwien.kr.alpha.core.parser.ParseTreeVisitor.visitNaf_literal(ParseTreeVisitor.java:465)
	at at.ac.tuwien.kr.alpha.core.parser.ParseTreeVisitor.visitBody(ParseTreeVisitor.java:312)
	at at.ac.tuwien.kr.alpha.core.parser.ParseTreeVisitor.visitStatement_rule(ParseTreeVisitor.java:204)
	at at.ac.tuwien.kr.alpha.core.antlr.ASPCore2Parser$Statement_ruleContext.accept(ASPCore2Parser.java:307)
	at org.antlr.v4.runtime.tree.AbstractParseTreeVisitor.visit(AbstractParseTreeVisitor.java:18)
	at at.ac.tuwien.kr.alpha.core.parser.ParseTreeVisitor.visitStatements(ParseTreeVisitor.java:176)
	at at.ac.tuwien.kr.alpha.core.parser.ParseTreeVisitor.visitProgram(ParseTreeVisitor.java:167)
	at at.ac.tuwien.kr.alpha.core.parser.ParseTreeVisitor.translate(ParseTreeVisitor.java:107)
	at at.ac.tuwien.kr.alpha.core.parser.ProgramParserImpl.parse(ProgramParserImpl.java:130)
	at at.ac.tuwien.kr.alpha.core.parser.ProgramParserImpl.parse(ProgramParserImpl.java:48)
	at at.ac.tuwien.kr.alpha.api.impl.AlphaImpl.readProgramString(AlphaImpl.java:129)
	at at.ac.tuwien.kr.alpha.api.impl.AlphaImpl.readProgram(AlphaImpl.java:99)
	at at.ac.tuwien.kr.alpha.Main.main(Main.java:87)

I looked through the code in question and GraalVM docs and found the following: The error occurs because Externals#getStandardLibraryExternals returns an empty map. This happens because Reflections#scan does not find any annotated methods in the classpath (see line 179 of Reflections.java in `reflections-0.9.11.jar).
This is consistent with the limitations of native images described here:

Any class to be accessed by name at image run time must be enumerated at image build time. For example, a call to Class.forName("myClass”) must have myClass in a configuration file.

I think to fix the issue it hand it should suffice to add AspStandardLibrary to the native-image configuration as described here.

Note that this raises a bigger issue: With alpha being compiled to a native binary, using user-supplied externals can't work the way it currently does anymore - Since class-loading doesn't happen the way we're used to, discovery of things on the cluasspath during runtime won't work.
Personally, I don't think we should treat this as a showstopper - The user-supplied externals case is more likely to occur in settings where Alpha is used as a part of a larger Java application, but I think we should keep it in mind and think about possible alternatives.

@lorenzleutgeb
Copy link
Member Author

I looked through the code in question and GraalVM docs and found the following: The error occurs because Externals#getStandardLibraryExternals returns an empty map. This happens because Reflections#scan does not find any annotated methods in the classpath (see line 179 of Reflections.java in `reflections-0.9.11.jar). This is consistent with the limitations of native images described here:

Any class to be accessed by name at image run time must be enumerated at image build time. For example, a call to Class.forName("myClass”) must have myClass in a configuration file.

I think to fix the issue it hand it should suffice to add AspStandardLibrary to the native-image configuration as described here.

Yes, reflection needs additional configuration to work with Native Image, I hinted that in #330 (comment). Probably should have been more explicit...

Note that this raises a bigger issue: With alpha being compiled to a native binary, using user-supplied externals can't work the way it currently does anymore - Since class-loading doesn't happen the way we're used to, discovery of things on the cluasspath during runtime won't work. Personally, I don't think we should treat this as a showstopper - The user-supplied externals case is more likely to occur in settings where Alpha is used as a part of a larger Java application, but [...]

Yes. Externals in Java won't work in the native image. I view the native image as a "conventional" ASP system, lacking externals.

[...] I think we should keep it in mind and think about possible alternatives.

True, but let's not just allow linking against arbitrary shared objects that supply implementations for externals...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants