[foreign] RFR 8222765: Implement foreign memory access through VarHandle
brian.goetz at oracle.com
Fri Apr 19 14:25:19 UTC 2019
It’s a little hard to tell exactly what this abstraction is. The class doc says “temporal and spatial bounds”; presumably the temporal bounds are determined by the scope. The methods `narrow()` and `offset()` hint at a spatial bound, but there’s no way to query the bound directly (no `size()` or `limit()` method), nor does it say what happens when offset/narrow violates the spatial bounds.
Is the model here that there is a hidden limit? And one can narrow it so long as the new limit is less than the current limit, and offset so long as we stay under the limit? And presumably, one can bind a layout to it, so long as there is room between the pointer and the limit for the layout?
I basically cannot tell whether this is the abstraction for “pointer” (subject to some bounds constraint) or “region” (describing a block of memory.), but it doesn’t quite feel like “address”. Maybe a less loaded name, like “MemoryRef”?
This feels mostly like an abstraction that does one thing, except for the method `confinementThread()`; this one definitely sticks out as “what’s this doing here.” I think I get how you got here — you don’t want to have a complex hierarchy of Scope types — and this was the one bit of extra information you needed to complete the story. But something feels a little “crammed” here.
Scope needs a method to expose its closed-state; presumably allocate() will fail if the scope is closed, but how do I check if it is closed before allocations?
There must be some rules for merging characteristics; presumably if the parent scope is confined, so must the child scopes.
General: the names for most of these classes are just too general — Address, Value, Sequence — users will curse us for polluting the namespace with things that look like what they want, but are not. (Just like java.awt.List.). There’s an easy fix: append Layout to all the concrete subtypes of Layout (SequenceLayout, GroupLayout, etc.). Similarly, Descriptor should be something like NativeDescriptor.
I get the sense all the types in this package are intended to be immutable / value-based. Should say that.
Is there a better name than “annotations”?
`stripAnnotations` sounds like a mutative method, but I think you intend implementations to be immutable? Maybe `withoutAnnotations`.
I think what this abstraction is, is a path from the root of a Layout to a particular sub-layout? And the enclosing() method provides the trail of breadcrumbs back to the root?
I wonder if there’s an easier way to express this. I assume what I’m trying to do is navigate to foo.bar[n].baz. But I don’t see how things like sequence indexes play into the story.
Having some trouble inferring the model from the API. Can we document the model separately?
> On Apr 19, 2019, at 9:32 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
> And here's a javadoc link in case somebody wants to do a pass over the API (30% feedback please :-))
> On 19/04/2019 14:25, Maurizio Cimadamore wrote:
>> this patch implements the MemoryAddress/MemoryScope abstractions described in the document I've sent recently :
>> This patch will go on new branch of Panama, called "foreign-memaccess", so the patch is relative to the 'default' branch.
>> Most of the classes added by this patch are the Layout classes we already have in the existing 'foreign' branch.
>> There are however a bunch of new classes: LayoutPath, MemoryAddress and MemoryScope (and their respective implementations).
>> In order to generate VarHandle with the right number of dimensions, we use a template (so that we can automatically generate all variants of VarHandle for all carriers). Then, we also need some bytecode spinning, since a concrete VarHandle might have a number of components indices which depend on the layout being accessed (more specifically, for each traversed array, there's an extra long component).
>> The spinning is done in the new AddressVarHandleGenerator class.
>> I've added a smoke test for the various memory mode accesses; we should of course add more tests e.g. to check that all scope options are properly enforced (such as confinement).
>> With this patch, performances of VH access is approx 15% slower than a plain Unsafe call. The main issue is that the scope liveness check is not hoisted outside of hot loops from hotspot (since the scope state is a mutable field which could be changed by a different thread). Vlad is working on the JIT optimization story for this; the idea is that when access is confined, JIT will be able to trust the fact that it's seeing all accesses to the scope it needs to and, provided the scope never escapes the inlined method, the liveness check can be hoisted, effectively bringing us on par with Unsafe.
>>  - http://cr.openjdk.java.net/~mcimadamore/panama/memaccess.html
More information about the panama-dev