[PING] RFR(M): 8207343: Automate vtable/itable stub size calculation
Schmidt, Lutz
lutz.schmidt at sap.com
Wed Aug 15 12:55:26 UTC 2018
Hi Vladimir,
what do you think about this change (see attachment) to convert tracing output to UL? I will create a new webrev once I have included your (expected) comments.
Regards, Lutz
On 15.08.18, 00:34, "Schmidt, Lutz" <lutz.schmidt at sap.com> wrote:
No hurry, Vladimir!
In the meantime, I will have a look at the PrintMiscellaneous to UL conversion. In the light of previous discussions, this might help making some people happy. __
Thanks,
Lutz
On 15.08.18, 00:25, "Vladimir Kozlov" <vladimir.kozlov at oracle.com> wrote:
On 8/14/18 12:46 AM, Schmidt, Lutz wrote:
> Hi Vladimir,
>
> the answer is simple: No, I did not. The proposed change is basically a generalized, harmonized variant of a modification we have been using at SAP for many years.
Okay. Give me time to look and test changes.
What do you think about using Xlog (UL) instead of PrintMiscellaneous in bookkeeping()?
>
> Your idea is interesting, though. I (still) do not completely like the implementation currently in RFR. The code size variance caused by data only known at runtime (constants, offsets, for example) makes the generators ugly.
>
> Using a temp buffer adds other complexities. What about relocations, for example. Without having had a deeper look, I expect the effort to be "considerable".
Yes, it is not simple change.
Thanks,
Vladimir
>
> Thanks,
> Lutz
>
> On 14.08.18, 00:49, "Vladimir Kozlov" <vladimir.kozlov at oracle.com> wrote:
>
> Hi Lutz,
>
> Did you consider to generate these stubs in temp buffer before publishing them in CodeCache as we do
> for nmethods?
>
> Thanks,
> Vladimir
>
> On 8/13/18 3:52 AM, Schmidt, Lutz wrote:
> > Dear Community,
> > are there any praises, objections, questions, comments, or even reviews for this change?
> > Thanks for considering!
> > Lutz
> >
> > On 13.08.18, 12:47, "Schmidt, Lutz" <lutz.schmidt at sap.com> wrote:
> >
> > Hi Boris,
> >
> > back from vacation I'd like to elaborate a bit more on your comments.
> >
> > Based on your input, I have changed the description in VtableStub::pd_code_alignment() to read
> > "ARM32 cache line size is not an architected constant. We just align on word size."
> >
> > The description for aarch64 was adapted accordingly:
> > "aarch64 cache line size is not an architected constant. We just align on 4 bytes (instruction size)."
> >
> > With respect to the variable name, I just harmonized the naming across all platforms. I would appreciate if you could live with it. Another option would be to just return a value, without using any variable name. I don't like that too much, but if the community prefers it...
> >
> > I have updated http://cr.openjdk.java.net/~lucy/webrevs/8207343.01 in-place with just the two comment lines modified.
> >
> > Thank you,
> > Lutz
> >
> > On 07.08.18, 22:12, "Schmidt, Lutz" <lutz.schmidt at sap.com> wrote:
> >
> > Hi Boris,
> > thanks for looking at this. I will respond to your comments in more detail next week. I'm on vacation this week.
> > Thanks,
> > Lutz
> >
> > On 05.08.18, 19:08, "Boris Ulasevich" <boris.ulasevich at bell-sw.com> wrote:
> >
> > Hi Lutz,
> >
> > I have run jtreg with your change and do not see new fails (test that
> > fails on aarch64 is excluded for 32 bit platforms).
> >
> > I am OK with your change (I'm not a reviewer). But description and
> > variable name in VtableStub::pd_code_alignment function looks strange
> > for me. Raspberry Pi2 ARM1176JZF-S processor has a cache line length of
> > 32 bytes, and, as I know, icache line size is not a constant for ARM32
> > architecture.
> >
> > regards,
> > Boris
> >
> > On 02.08.2018 15:02, Schmidt, Lutz wrote:
> > > Hi Zhongwei,
> > >
> > > thank you for testing aarch64. Given my lack of expertise, I am surprised there are only those two issues.
> > > Ad 1.: fixed. The declaration just a few lines above was forgotten to adapt.
> > > Ad 2.: adapted. I pushed the estimate to 152 bytes. I expected this value would require some adjustment.
> > >
> > > Please find a new webrev with the changes at http://cr.openjdk.java.net/~lucy/webrevs/8207343.01
> > >
> > > Best Regards,
> > > Lutz
> > >
> > > On 02.08.18, 04:45, "Zhongwei Yao" <Zhongwei.Yao at arm.com> wrote:
> > >
> > > Hi, Lutz,
> > >
> > > I have tested it on aarch64 by running jtreg tests. And find two tiny issues in vtableStubs_aarch64.cpp's VtableStubs::create_itable_stub function:
> > > 1. typecheckSize on line 212 is not defined.
> > > 2. estimate on line 211 is not large enough, I get 148 in gc/g1/TestFromCardCacheIndex.java case. Here is the assertion failure from that case:
> > > assert(slop_delta >= 0) failed: itable #3: Code size estimate (140) for lookup_interface_method too small, required: 148
> > >
> > > However, I don't have tested the modification in vtableStubs_arm.cpp due to I don't have an arm32 environment at hand.
> > >
> > > --
> > > Best regards,
> > > Zhongwei
> > >
> > > ________________________________________
> > > From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net> on behalf of Schmidt, Lutz <lutz.schmidt at sap.com>
> > > Sent: Monday, July 30, 2018 3:57:05 PM
> > > To: hotspot-compiler-dev at openjdk.java.net
> > > Subject: RFR(M): 8207343: Automate vtable/itable stub size calculation
> > >
> > > Dear all,
> > >
> > > may I please request reviews for this change:
> > >
> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8207343
> > > Webrev: http://cr.openjdk.java.net/~lucy/webrevs/8207343.00/
> > >
> > > With this change, I try to get rid of the a-priory size guessing for vtable and itable stubs. Please refer to the bug description for all the details. I didn't want to duplicate that text.
> > >
> > > ARM and AARCH64 help requested!
> > > The edits in vtableStubs_aarch64.cpp and vtableStubs_arm.cpp are made blindfolded. I am neither an ARM expert nor do I have build or test hardware available. I would be very grateful if one of the ARM gurus could please fill in for me.
> > >
> > > Thank you!
> > > Lutz
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> >
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ULTransition.diff
Type: application/octet-stream
Size: 2947 bytes
Desc: ULTransition.diff
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20180815/46eb83b7/ULTransition-0001.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ULTransition.log
Type: application/octet-stream
Size: 4604 bytes
Desc: ULTransition.log
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20180815/46eb83b7/ULTransition-0001.log>
More information about the hotspot-compiler-dev
mailing list