RFR: 8229258: Rework markOop and markOopDesc into a simpler mark word value carrier

[Doerr, Martin]

Hi Stefan,

> 1) Platform maintainers probably want to run this patch through their
> build system.
builds on PPC64 and s390.

Best regards,
Martin


> -----Original Message-----
> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf Of
> coleen.phillimore at oracle.com
> Sent: Donnerstag, 15. August 2019 14:57
> To: hotspot-dev at openjdk.java.net
> Subject: Re: RFR: 8229258: Rework markOop and markOopDesc into a simpler
> mark word value carrier
> 
> 
> https://cr.openjdk.java.net/~stefank/8229258/webrev.valueMarkWord.03.d
> elta/src/hotspot/share/runtime/synchronizer.cpp.udiff.html
> 
> + lock->set_displaced_header(markWord::from_pointer(NULL));
> 
> 
> Should this be markWork::zero ?
> 
> + m->set_header(markWord(markWord::zero));
> 
> 
> ew.
> 
> I just clicked through the latest incremental and the full oops directory.
> 
> The to_pointer and from_pointer casts are awkward, particularly because
> you can do (Edge*)mark.value() too, but it's good to find where we're
> using the markWord for various purposes.
> 
> This is a massive improvement.  Dont' change a thing and check it in.
> We can do minor cleanups in followup RFEs.
> 
> Thanks!
> Coleen
> 
> 
> On 8/15/19 7:46 AM, Stefan Karlsson wrote:
> > Thanks Kim, Roman, Dan and Coleen for reviews and feedback.
> >
> > I rebased the patch, fixed more alignments, renamed the bug, and rerun
> > the test through tier1-3.
> >
> >
> https://cr.openjdk.java.net/~stefank/8229258/webrev.valueMarkWord.03.d
> elta/
> >
> >
> https://cr.openjdk.java.net/~stefank/8229258/webrev.valueMarkWord.03/
> >
> > Could I get reviews for this version? I'd also like to ask others to
> > at least partially look at this:
> >
> > 1) Platform maintainers probably want to run this patch through their
> > build system.
> > 2) SA maintainers (CC:ed serviceability-dev)
> > 3) JVMCI maintainers
> >
> > Thanks,
> > StefanK
> >
> > On 2019-08-14 11:11, Roman Kennke wrote:
> >>
> >> Am 14.08.19 um 01:26 schrieb Kim Barrett:
> >>>> On Aug 12, 2019, at 12:19 PM, Stefan Karlsson
> >>>> <stefan.karlsson at oracle.com> wrote:
> >>>>
> >>>> Hi Roman,
> >>>>
> >>>> Kim helped me figuring out how to get past the volatile issues I
> >>>> had with the class markWord { uintptr_t value; ... } version. So,
> >>>> I've created a version with that:
> >>>>
> >>>>
> https://cr.openjdk.java.net/~stefank/8229258/webrev.valueMarkWord.01/
> >>>>
> >>>> I can go with either approach, so let me now what you all think.
> >>> I've finally had time to look at the first proposed change.
> >>>
> >>> Comparing the first approach (an AllStatic MarkWord class and
> markWord
> >>> typedef'd to uintptr_t) vs the second approach (markWord is a thin
> >>> class wrapping around uintptr_t), I prefer the second.
> >>>
> >>> * I think the markWord class provides better type safety. It still
> >>> involves too many casts sprinkled over the code base, but I think it
> >>> also provides a better basis for further cast reduction and
> >>> prevention.
> >>>
> >>> * I think having one markWord class for the data and behavior is
> >>> better / more natural than having a markWord typedef for the data and
> >>> a MarkWord AllStatic class for the behaviour.
> >>>
> >>> * I like that the markWord class eliminates the markWord vs MarkWord
> >>> homonyms, which I think will be annoying.
> >>>
> >>> * The markWord class is a trivially copyable class, allowing it to be
> >>> efficiently passed around by value, so no disadvantage there.
> >>>
> >>> I haven't found anything that I think argues for the first over the
> >>> second. Other folks might have different priorities or taste. I think
> >>> either is better than the status quo.
> >>>
> >>> I'm still reviewing webrev.valueMarkWord.02, but so far haven't found
> >>> anything that makes me want to suggest backing off from that direction.
> >>>
> >>> Note that the bug summary doesn't describe the second approach.
> >> +1 :-)
> >>
> >> Roman
> >>
> >