HostObject interop with wrapping TruffleObject #4916

JaroslavTulach · 2022-09-10T07:47:36Z

Enso language supports values with warnings. They are implemented as a TruffleObject that wraps original object, holds the warnings and delegates all messages via ReflectionLibrary to the original object. Find more details in the implementation.

This PR provides a test showing the use-case and demonstrating the failure as the first commit. Additional commits follow showing possible implementation making the test work: 629d3df

oracle-contributor-agreement · 2022-09-10T07:47:41Z

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:

PR author: @JaroslavTulach

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

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

JaroslavTulach · 2022-09-10T07:54:26Z

....oracle.truffle.api.test/src/com/oracle/truffle/api/test/polyglot/HostObjectWrappedTest.java

+            Value wrapFive = WrapObject.wrap(five);
+
+            Value wrapPlus = four.invokeMember("add", wrapFive);
+            assertEquals(444L + 555L, wrapPlus.invokeMember("longValue").asLong());


When the test is executed on 21.3 branch, it fails with:

1) com.oracle.truffle.api.test.polyglot.HostObjectWrappedTest#testHostInteropWithWrapper java.lang.IllegalArgumentException: Invalid argument when invoking 'add' on '444'(language: Java, type: java.math.BigInteger). Cannot convert '555'(language: Java, type: java.math.BigInteger) to Java type 'java.math.BigInteger': Unsupported target type.Provided arguments: ['555'(language: Java, type: java.math.BigInteger)]. at com.oracle.truffle.polyglot.PolyglotEngineException.illegalArgument(PolyglotEngineException.java:125) at com.oracle.truffle.polyglot.PolyglotValueDispatch.invalidInvokeArgumentType(PolyglotValueDispatch.java:1432)

chumer · 2022-09-12T09:48:38Z

Are you using WrapObject.wrap in production code, if so why?

I agree the kind of refactoring sketched in 6b9dfae would roughly do the trick. We cannot however just use uncached everywhere, that would lead to a performance regression. We would need to create cached libraries and use them where appropriate. We also have some dependent EE code we would need to fix on our side for this.

JaroslavTulach · 2022-09-12T10:09:33Z

Are you using WrapObject.wrap in production code, if so why?

No, that's just for the test. The production code has EnsoLanguage extends TruffleLanguage and obtains TruffleObject via lookupHostSymbol. I can rewrite the test to avoid reflection, if I get guidance what to copy. I have found tests for HostObject and tests for looking Java classes up, but none that mixes both together.

JaroslavTulach

We cannot however just use uncached everywhere, that would lead to a performance regression

Whenever there is a Node "somewhere", the patch uses cached version. The goal is to use uncached only when behind @TruffleBoundary or in an @Override method (overridden methods are usually not subject to PE, right?). Also when there already is InteropLibrary.getUncached() or etc., it is OK to use HostLibrary.getUncached().

truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostExecuteNode.java

JaroslavTulach · 2022-09-12T13:25:09Z

truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostLanguageService.java

@@ -152,7 +152,7 @@ private boolean validHostValue(Object hostValue, HostContext context) {

    @Override
    public boolean isHostValue(Object value) {
-        Object obj = HostLanguage.unwrapIfScoped(language, value);
+        Object obj = HostLanguage.unwrapIfScoped(language, HostLibrary.getUncached(), value);


Using HostLibrary.getUncached() as isHostValue() is an @Override method. Overridden methods are (usually) not subject to PE.

That is not true. Such a rule does not exist and does particularly not apply for most of the truffle.polyglot and truffle.host code. The only important thing is that the virtual call can be devirtualized reliably during PE otherwise it would cause a performance warning during compilation. This is generally true for all objects rooted in the PolyglotEngineImpl class like AbstractPolyglotHostService as the engine is always a constant in PE paths.

Note that AbstractPolyglotHostService exists for polyglot isolates, as all host operations need to be handled differently there.

The @Override claim, was just a (wrong) guess. I know PE doesn't like overwritten methods, but if it resolves to a single method, then it of course works fine.

Update on Sep 19:
The Old Blocklisted method check does no longer work after updating to 22.x master branch. Using new trick: let's add neverPartOfCompilation() to HostLibrary.getUncached(): 77254c3 - then @tzezula suggested me to run following tests:

graal/compiler$ mx unittest --verbose com.oracle.truffle

Obviously they failed on compilations. Fixed by 72e0162 - plus I've added two sets of @TruffleBoundary annotations:

asGuestValue: 1018742

NFI usage: 272d979

Fixing those two would require changes in the API. I Hope this is good enough base for future work that can be adopted by Truffle team lands in GraalVM soonish.

Old Blocklisted method check:

To compensate for my erroneous judgment, I suggest following check: Modify the unwrapIfScoped method to reference some blacklisted method:

graal/vm$ git diff diff --git a/truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostLanguage.java b/truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostLanguage.java index 9706e5f5e56..a045c0ce7b0 100644 --- a/truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostLanguage.java +++ b/truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostLanguage.java @@ -61,6 +61,7 @@ import com.oracle.truffle.api.interop.UnsupportedMessageException; import com.oracle.truffle.api.nodes.Node; import com.oracle.truffle.api.nodes.RootNode; import com.oracle.truffle.host.HostMethodScope.ScopedObject; +import java.io.File; /* * Java host language implementation. @@ -94,6 +95,7 @@ final class HostLanguage extends TruffleLanguage<HostContext> { } static Object unwrapIfScoped(HostLanguage language, HostLibrary library, Object o) { + if (!new File(".").exists()) throw new IllegalStateException(); HostInstance host = library.asHostInstance(o); if (language == null || !language.methodScopingEnabled) { return host;

and then compile js with native image. It gives seven violations like:

graal/vm$ mx --native-image=js --java-home ~/bin/labs-11/ --dy=/graal-js,/espresso --env ni-ce build === Found 7 compilation blocklist violations === Blocklisted method java.io.UnixFileSystem.getBooleanAttributes0(File) called from java.io.UnixFileSystem.getBooleanAttributes(UnixFileSystem.java:248) java.io.File.exists(File.java:831) com.oracle.truffle.host.HostLanguage.unwrapIfScoped(HostLanguage.java:98) com.oracle.truffle.host.HostObject.isInstance(HostObject.java:147) -> com.oracle.truffle.host.HostLanguageService.isHostObject(HostLanguageService.java:203) com.oracle.truffle.polyglot.EngineAccessor$EngineImpl.isHostObject(EngineAccessor.java:1080) com.oracle.truffle.api.TruffleLanguage$Env.isHostObject(TruffleLanguage.java:2116) com.oracle.truffle.js.builtins.ConstructorBuiltins$ConstructJavaImporterNode.constructJavaImporter(ConstructorBuiltins.java:2233) com.oracle.truffle.js.builtins.ConstructorBuiltinsFactory$ConstructJavaImporterNodeGen.execute(ConstructorBuiltinsFactory.java:4490) com.oracle.truffle.js.nodes.control.TopLevelAwaitModuleBodyNode$TopLevelAwaitModuleRootNode.execute(TopLevelAwaitModuleBodyNode.java:106) org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.executeRootNode(OptimizedCallTarget.java:650) org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.profiledPERoot(OptimizedCallTarget.java:622)

All of them go thru com.oracle.truffle.api.TruffleLanguage$Env.isHostObject.

truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostObject.java

truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostTargetMappingNode.java

truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostToTypeNode.java

… HostObject interop

…to HostObject and co.

…TruffleLanguage.Env

chumer · 2022-10-04T08:17:00Z

truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostInstance.java

+
+/** Marker interface to recognize all host objects.
+ */
+interface HostInstance {


Why is the purpose of this?

chumer · 2022-10-04T08:25:54Z

truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostLibrary.java

+    }
+
+    public HostInstance asHostInstance(Object obj) {
+        return obj instanceof HostInstance ? (HostInstance) obj : null;


Interface type checks are very expensive. Any reason why there is not a isHostInstance and asHostInstance?
The first returns true|false whether it is a host object. And the second returns not the HostObject wrapper but the underlying real host object. asHostInstance then could fail with an UnsupportedMessageException if it is used with the wrong implementation.

We should let ScopedObject implement HostLibrary as well that significantly improves other code such that we can get rid of HostLanguage.unwrapIfScoped.

dougxc · 2022-10-10T07:36:18Z

@JaroslavTulach has now signed an OCA.

JaroslavTulach · 2023-01-31T05:09:50Z

I have to put this PR on hold as I don't have time to finish it right now.

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

JaroslavTulach commented Sep 10, 2022

View reviewed changes

JaroslavTulach commented Sep 12, 2022

View reviewed changes

Jaroslav Tulach added 2 commits September 16, 2022 15:44

HostObjectWrappedTest demonstrating issues with ReflectionLibrary and…

7eb0b22

… HostObject interop

Introducing HostLibrary to allow ReflectiveLibrary to unwrap objects …

4be5fe5

…to HostObject and co.

JaroslavTulach force-pushed the jtulach/WrapHostObjectInterop branch from 629d3df to 4be5fe5 Compare September 16, 2022 13:58

JaroslavTulach changed the base branch from release/graal-vm/21.3 to master September 19, 2022 05:25

Jaroslav Tulach added 4 commits September 19, 2022 09:03

Only use HostLibrary.getUncached() on slow path

77254c3

Let asGuestValue run behind @TruffleBoundary

1018742

Make sure fast-path access is using cached HostLibrary

72e0162

Hiding behind @TruffleBoundary - this would require an API change in …

272d979

…TruffleLanguage.Env

chumer reviewed Oct 4, 2022

View reviewed changes

JaroslavTulach mentioned this pull request Oct 4, 2022

Fix Warnings visualization enso-org/enso#3754

Merged

4 tasks

oracle-contributor-agreement bot added OCA Verified All contributors have signed the Oracle Contributor Agreement. and removed OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. labels Oct 10, 2022

JaroslavTulach closed this Jan 31, 2023

wdanilo mentioned this pull request Feb 6, 2023

Truffle Interop Types: Array & Vectors enso-org/enso#4542

Closed

8 tasks

JaroslavTulach mentioned this pull request Jan 5, 2024

Support on_problems=Problem_Behavior.Report_Warning and Map_Error wrapping in Vector.map enso-org/enso#8595

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HostObject interop with wrapping TruffleObject #4916

HostObject interop with wrapping TruffleObject #4916

JaroslavTulach commented Sep 10, 2022 •

edited

Loading

oracle-contributor-agreement bot commented Sep 10, 2022

JaroslavTulach Sep 10, 2022

chumer commented Sep 12, 2022

JaroslavTulach commented Sep 12, 2022

JaroslavTulach left a comment

JaroslavTulach Sep 12, 2022

chumer Sep 13, 2022

chumer Sep 13, 2022

JaroslavTulach Sep 13, 2022 •

edited

Loading

chumer Oct 4, 2022

chumer Oct 4, 2022 •

edited

Loading

dougxc commented Oct 10, 2022

JaroslavTulach commented Jan 31, 2023

HostObject interop with wrapping TruffleObject #4916

HostObject interop with wrapping TruffleObject #4916

Conversation

JaroslavTulach commented Sep 10, 2022 • edited Loading

oracle-contributor-agreement bot commented Sep 10, 2022

JaroslavTulach Sep 10, 2022

Choose a reason for hiding this comment

chumer commented Sep 12, 2022

JaroslavTulach commented Sep 12, 2022

JaroslavTulach left a comment

Choose a reason for hiding this comment

JaroslavTulach Sep 12, 2022

Choose a reason for hiding this comment

chumer Sep 13, 2022

Choose a reason for hiding this comment

chumer Sep 13, 2022

Choose a reason for hiding this comment

JaroslavTulach Sep 13, 2022 • edited Loading

Choose a reason for hiding this comment

chumer Oct 4, 2022

Choose a reason for hiding this comment

chumer Oct 4, 2022 • edited Loading

Choose a reason for hiding this comment

dougxc commented Oct 10, 2022

JaroslavTulach commented Jan 31, 2023

JaroslavTulach commented Sep 10, 2022 •

edited

Loading

JaroslavTulach Sep 13, 2022 •

edited

Loading

chumer Oct 4, 2022 •

edited

Loading