graal icon indicating copy to clipboard operation
graal copied to clipboard

Give host code a chance to create non-modifiable polyglot array

Open JaroslavTulach opened this issue 2 years ago • 7 comments

We are working on an implementation of a functional programming language on top of Truffle. One of the important features of functional languages is referential transparency. We can reach it in our language, but we have problems achieving it effectively when dealing with other languages across polyglot boundary.

For example, when we receive an object which hasArrayElements, we'd like to know whether it is safe to use it as is, or whether we shall defensively copy its content. Usually we have both sides (the Enso one and the Java one) under our control. E.g. the problem is just about communicating between these two whether the array can be considered immutable or not.

It has come to my attention that there is isArrayElementModifiable which we could use to ask the array. Assuming that if all elements we read are not modifiable, then the array won't change. I consider it a good heuristic.

However then I tried to investigate what needs to be done on the (host) Java side to mark an array (or rather a List) as read only. I've noticed there is no such way (probably the isArrayElementXyz messages were added later than polyglot SDK and nobody needed to expose them yet). This PR is my attempt to allow (host) Java to express that a List is not mutable by using check for friendly class implementations. I don't think the current code is mergable, but it should be good enough to start discussion.

What do you think, @chumer, @tzezula, @jdunkerley, @kustosz, @radeusgd, @hubertp?

JaroslavTulach avatar Aug 08 '22 07:08 JaroslavTulach

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.

When singing the OCA

Cute. Óóó sééé Ej!

JaroslavTulach avatar Aug 10 '22 10:08 JaroslavTulach

isArrayElementWritable false only tells you that an element is not writable, not that it is immutable like you assume it. There is no immutable contract in interop you could reliably use for that. So I do not think you will get around calling into a host-specific method to find out whether the List is immutable according to host Java. The way you do this, by comparing the class, is also not really a specified way(or is it?), so I think you are on your own there.

I think we do have a bug here though. I see that WriteArrayElement#doList currently does not currently catch UnsupportedOperationException. It should probably throw an UnsupportedMessageException for UnsupportedOperationException.

chumer avatar Aug 24 '22 09:08 chumer

only tells you that an element is not writable, not that it is immutable

Yup, I know, it is just my interpretation.

The way you do this, by comparing the class, is also not really a specified way(or is it?)

Comparing implementation class is already used in the host interop package: https://github.com/oracle/graal/blob/01cdd0ac7b496711b8afd1dbc6649e335c4f6465/truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostObject.java#L104

JaroslavTulach avatar Aug 25 '22 09:08 JaroslavTulach

However the real question for SDK & Truffle API:

isArrayElementWritable false only tells you that an element is not writable

How does one create an object that hasArrayElements and (some of its) fields are isArrayElementWritable == false in host Java? Looks to me Collections.unmodifyiableList isn't enough right now, neither ProxyArray is enough...

Doing the implementation class check is one way. Having an ProxyReadOnlyArray interface another one that comes to my mind. Any contract will do, but right now there is just no way.

JaroslavTulach avatar Aug 25 '22 09:08 JaroslavTulach

Comparing implementation class is already used in the host interop package:

That is done for performance reasons only. It's not observable by the user, as it would be if isArrayElementWritable were implemented that way.

How does one create an object that hasArrayElements and (some of its) fields are isArrayElementWritable == false in host Java?

Not everything is exposed to host java. So you cannot directly influence every interop message. But the semantics of interop are that you check whether isArrayElementWritable and then you call into writeArrayElement. WriteArrayElement is then still allowed to throw an UnsupportedMessageException. This can of course, have slightly different semantics in the languages, but in the proxy interfaces, there is only the latter way available, that unfortunately is currently bugged. As already mentioned.

Doing the implementation class check is one way. Having an ProxyReadOnlyArray interface another one that comes to my mind. Any contract will do, but right now there is just no way.

One idea: Could you only implement ProxyIterable as a workaround? Or do you need array semantics?

I think I prefer ProxyReadOnlyArray. We could introduce ProxyReadOnlyArray as a base class to ProxyArray. That should be compatible, right?

chumer avatar Aug 25 '22 15:08 chumer

Doing the implementation class check is one way. Having an ProxyReadOnlyArray interface another one that comes to my mind. Any contract will do, but right now there is just no way.

One idea: Could you only implement ProxyIterable as a workaround? Or do you need array semantics?

Enso base type is Vector with at method. As such we need array semantics without needless copying.

I think I prefer ProxyReadOnlyArray. We could introduce ProxyReadOnlyArray as a base class to ProxyArray. That should be compatible, right?

Adding a super interface ProxyReadOnlyArray with subset of ProxyArray methods would be compatible change. I'll see whether I can prepare it soon.

JaroslavTulach avatar Sep 07 '22 13:09 JaroslavTulach

At the end we plan to consolidate Vector & Array rather than trying to differentiate between them.

JaroslavTulach avatar Oct 04 '22 06:10 JaroslavTulach