RFC: Refactoring SystemABI

[Henry Jen]

> On Dec 10, 2018, at 4:06 PM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
> 
> Hi,
> I think this is a lot closer to what I had in mind.
> 
> If I'm correct there are only two usages of JavaSymbol:
> 
> * ScopeImpl creates one using JavaSymbol::exportTo
> * Scope stores them into a List, and then frees them when the scope is closed
> 
> I think that, JavaSymbol serves little purpose now, and can be removed. You can just call the ABI code directly to do whatever JavaSymbol::exportTo does, and, similarly, you can do the same for closing.
> 

True. In that case, we pretty much back to .01. JavaSymbol is just a higher level construct to give a better symbol name and correct Pointer scope. But we don’t really need it since we only use pointer address to get back the UpcallStub.

> Your new UpcallStub is what I really would call JavaSymbol - and this type is entirely under the water - it's completely ABI specific (and should live in the 'abi' package). So, I think it is better, but the improvement is somewhat hidden by the fact that the 'invokers' live in their own separate package, while they should really live in the 'abi' package (in fact the binder doesn't 'need' them anymore).
> 

ABI specific means SysV specific or ABI in common? That decided where that should go. Move .invokers into .abi seems reasonable to me. JavaSymbol to me need to have real name of Java class/method name. :)

Cheers,
Henry


> Other than that I think it looks good?
> 

> Maurizio
> 
> On 10/12/2018 23:50, Henry Jen wrote:
>> I took another hack to push down the search down into SysVx64ABI so that ABI implemetnation is not to be aware to JavaSymbol, but in order to have MethodHandle in CodeBlob, we sort of have UpcallHandle back.
>> 
>> Take a look at webrev.04[1] or the diff[2] between .03 and .04, not sure .04 is much better. After all, JavaSymbol is internal although it is indeed a level above ABI and have SysVx64ABI be aware of it doesn’t feel “clean”.
>> 
>> Let me know what you think. At this point, I kind of think it better we pushed either .03 or .04, at least we have SystemABI interface finalized.
>> 
>> Cheers,
>> Henry
>> 
>> [1] http://cr.openjdk.java.net/~henryjen/panama/SystemABI/webrev.04/webrev/
>> [2] http://cr.openjdk.java.net/~henryjen/panama/SystemABI/diff-03-04/webrev/
>> 
>> 
>>> On Dec 10, 2018, at 8:58 AM, Henry Jen <henry.jen at oracle.com> wrote:
>>> 
>>>> On Dec 10, 2018, at 3:16 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>>>> 
>>>> This is looking better thanks.
>>>> 
>>>> I think JavaSymbol is still doing more than it should, and a sign of that is the fact that now you need an hash table to keep track of the allocated stubs in order to find them. This is a new requirement that was not present in the old code, and is introduced by the fact that, I think, we are trying to look 'too hard' for whether a pointer corresponds to an existing JavaSymbol.
>>>> 
>>> Yes and No. There are two thing I am not satisfied with this patch, one is the map used to find JavaSymbol from a Pointer and the other is we have to save a copy of Library.Symbol returned from SystemABI for free later.
>>> 
>>> The first dislike, as you have explained later, sort of duplicate to native side where we can search for a CodeBlob. I think we can push that down to SysVx64ABI, but not sure how much that search will be better while we can do it earlier.
>>> 
>>> Now, from the pointer to MH, you either need a NMT to have SystemABI make it, or trying to “stay above” by getting an UpcallHandle, this is basically now a JavaSymbol.
>>> 
>>> The extra stub, well, we need to wrap up the pointer with socpe. But SystemABI upcallHandle now return a symbol, we should not be assuming any symbol with same pointer can do, so we have to save it for free later.
>>> 
>>> Rest of code in JavaSymbol, we will have to do the same for ScopeImpl, either in JavaSymbol or somewhere, JavaSymbol seems to give a good sensible place for those.
>>> 
>>> Adding a Kind enum and a instanceof operation, I don’t see the benefit. Personally, I think current approach is good, and perhaps when we open up with other language, we could have NashornSymbol or JRubySymbol.
>>> 
>>> Make sense?
>>> 
>>> Cheers,
>>> Henry
>>>