----- Mail original ----- > De: "Jorn Vernee" <jorn.ver...@oracle.com> > À: "Maurizio Cimadamore" <maurizio.cimadam...@oracle.com>, "mandy chung" > <mandy.ch...@oracle.com>, "Remi Forax" > <fo...@univ-mlv.fr> > Cc: "core-libs-dev" <core-libs-dev@openjdk.java.net> > Envoyé: Mercredi 29 Avril 2020 22:09:47 > Objet: Re: RFR 8243491: Implementation of Foreign-Memory Access API (Second > Incubator)
> Hi, Hi Jorn, > > 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, > > 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. > > Given the amount of work, I'd say definitely something that should be > saved for another time, also since there doesn't appear to be a major > payoff for doing that at the moment. I think it's either to have a small class instead of an array with all the fields marked as @Stable, this will also avoid the cast because the fields will have the right type. > > Jorn Rémi > > On 29/04/2020 03:13, Maurizio Cimadamore wrote: >> >> On 28/04/2020 21:44, Maurizio Cimadamore wrote: >>> >>> On 28/04/2020 19:09, Mandy Chung wrote: >>>> On 4/28/20 12:58 AM, fo...@univ-mlv.fr wrote: >>>>> >>>>> >>>>> >>>>> >>>>> I don't think you need to store all the values into static >>>>> fields, you can directly do a ldc + aaload with the right index >>>>> right where you need it, >>>>> >>>>> >>>>> I think this is what you are thinking as reported in JDK-8243492: >>>>> http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8239578/webrev.01-accessor/ >>>>> >>>>> >>>>> >>>>> if the accessors are declared ACC_STATIC, yes ! >>>>> >>>> >>>> Thanks for catching this and this way will not be hit JDK-824349. >>>> >>>> Here is the revised patch: >>>> http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8239578/webrev.02/ >>>> >>>> Maurizio - do you mind remerge MemoryAccessVarHandleGenerator.java >>>> with webrev.02? >>> >>> I'll take care of that >> >> 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). >> >> Cheers >> Maurizio >> >>> >>> Maurizio >>> >>>> >>>> thanks > >>> Mandy
