[RFR] Instrumentation for Unsafe CAS

[Jean Christophe Beyler]

Hi Man,

Looks good to me too :)
Jc

On Mon, Sep 23, 2019 at 3:12 PM Arthur Eubanks <aeubanks at google.com> wrote:

> On Mon, Sep 23, 2019 at 2:58 PM Man Cao <manc at google.com> wrote:
>
> > Looks good.
> > Last nit, no need for webrev:
> >
> > ScopedReleaseAcquire(oop obj, ptrdiff_t offset) {
> >
> > This could use "jlong offset" now.
> >
> > Done.
>
> > -Man
> >
> >
> >
> > On Mon, Sep 23, 2019 at 2:50 PM Arthur Eubanks <aeubanks at google.com>
> > wrote:
> >
> >> New webrev: http://cr.openjdk.java.net/~aeubanks/tsancas/webrev.02
> >>
> >> On Mon, Sep 23, 2019 at 2:30 PM Man Cao <manc at google.com> wrote:
> >>
> >>> My main comment is:
> >>>
> >>> +  ScopedReleaseAcquire(oop obj, ptrdiff_t offset):
> addr_(AccessInternal::field_addr(obj, offset)) {
> >>>
> >>> This could use "index_oop_from_field_offset_long(obj, offset)", right?
> >>>
> >>> Went back to the original version but used
> >> index_oop_from_field_offset_long() instead of
> AccessInternal::field_addr().
> >>
> >>> Also discussed with Arthur that when TSAN is disabled at build-time,
> the code would still create the ScopedReleaseAcquire object, but hopefully
> the C++ compiler will optimize out no-op objects.
> >>>
> >>> Minor comments:
> >>>
> >>> +class ScopedReleaseAcquire {+private:+  void* addr_;
> >>>
> >>> This class should inherit from "public StackObj".
> >>>
> >>> Done
> >>
> >>> HotSpot style guide <
> https://wiki.openjdk.java.net/display/HotSpot/StyleGuide#StyleGuide-Names>
> says instance variable name should start with _, so "_addr".
> >>>
> >>> Done
> >>
> >>> -Man
> >>>
> >>>
> >>>
> >>> On Mon, Sep 23, 2019 at 12:52 PM Arthur Eubanks <aeubanks at google.com>
> >>> wrote:
> >>>
> >>>> webrev:
> >>>> http://cr.openjdk.java.net/~aeubanks/tsancas/webrev.00/index.html
> >>>>
> >>>> This is fairly self explanatory. I went down the route of using a C++
> >>>> class's constructor/destructor for the release/acquire so I didn't
> have
> >>>> to
> >>>> touch the existing code too much.
> >>>>
> >>>
>


-- 

Thanks,
Jc