Hi Remi,

Thanks for the feedback.  We can take this off from this review thread and roll it into JDK-8230501.

Mandy

On 4/30/20 11:03 AM, fo...@univ-mlv.fr wrote:
Hi Mandy,
i've taken a look to the code,
i think it's better to have two methods, one for List and one for Map to avoid to have a bootstrap argument (classDataType) and to have a code more straightforward.

Rémi

------------------------------------------------------------------------

    *De: *"mandy chung" <mandy.ch...@oracle.com>
    *À: *"Remi Forax" <fo...@univ-mlv.fr>, "Jorn Vernee"
    <jorn.ver...@oracle.com>
    *Cc: *"Maurizio Cimadamore" <maurizio.cimadam...@oracle.com>,
    "core-libs-dev" <core-libs-dev@openjdk.java.net>
    *Envoyé: *Jeudi 30 Avril 2020 01:05:46
    *Objet: *Re: RFR 8243491: Implementation of Foreign-Memory Access
    API (Second Incubator)



    On 4/29/20 2:30 PM, fo...@univ-mlv.fr wrote:

            I think the problem with perf might be caused by the fact that 
while the
            array is now a constant, the elements are not (the array is mutable
            after all). For fields you can fix this by using @Stable, but not 
for CP
            entries :)

        I think you're right,


    Ah, I missed that!

            I think what could work is; rather than ldc'ing the array, and then
            looking up the values with 'normal' Java code, we could have another
            dynamic constant that does the the array lookup as well. Then the
            resolved value is stored in a separate CP slot as a true constant. 
We
            probably want to have a bootstrap method in ConstantBootstraps that 
can
            do an arbitrary array lookup given an array and an index for that.


    FYI.  I'm exploring is `classDataAt` to get a specific
    element/entry from a class data of immutable List or Map.

    [1]
    
http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8239578/webrev.00/src/java.base/share/classes/java/lang/invoke/MethodHandles.java.frames.html

                Not going down this road, sorry :-)

                I've added the changes (see attached patch), and all benchmarks 
are
                several order of magnitude slower. I think this is mostly 
caused by
                the fact that the addOffset/multiplyOffset handle are no  longer
                cached in static constants.

                While I understand that there might be better ways to generate 
the
                code, I'd strongly prefer to leave the code as per last 
iteration. I
                can't honestly see in which way having 3-4 static fields in a
                synthetic VarHandle class is going to hurt (but I can see how 
much it
                hurts by not having said fields).



    I agree to keep the code per last iteration.  We can always
    improve this in the future with performance measurement.

    Mandy



Reply via email to