Shenandoah Events

[zgu at redhat.com]

Hi Ken,

On Fri, 2019-01-18 at 16:02 -0500, Ken Dobson wrote:
> Actually I've just gotten my authorship so here is a link to the
> webrev.
> 
> http://cr.openjdk.java.net/~kdobson/serializer/webrev/
> 
What's the status about this patch? Is it ready to go in? Do you need
help to create RFE and sponsor?

Thanks,

-Zhengyu

> 
> On Fri, Jan 18, 2019 at 12:43 PM Aleksey Shipilev <shade at redhat.com>
> wrote:
> > On 1/18/19 5:02 PM, Ken Dobson wrote:
> > > On Wed, Jan 16, 2019 at 3:40 PM Aleksey Shipilev <shade at redhat.co
> > m <mailto:shade at redhat.com>> wrote:
> > >     *) Not sure why some paths have JFR_ONLY, and some do not.
> > Try to configure build with
> > >     --with-jvm-features=-jfr and build? Pretty sure you would
> > need to put something like NO_JFR_RETURN
> > >     to send_jfr_region_transition_event declaration.
> > > 
> > > I think JFR_ONLY is for when you call just a single jfr related
> > function in a section of the code
> > > that will still be run with JFR removed. It removes the code in
> > that case. Is this what you mean by
> > > NO_JFR_RETURN?
> > 
> > There are two ways to conditionalize on JFR flag:
> > 
> > a) Declare the function with NO_JFR_RETURN:
> > 
> > blah.hpp:
> >   void my_jfr_event_handler() NO_JFR_RETURN;
> > 
> > blah.cpp:
> >   #if INCLUDE_JFR
> >    void my_jfr_event_handler() {
> >      ...
> >    }
> >   #endif
> > 
> > use.cpp:
> >   m() {
> >     my_jfr_event_handler();
> >   }
> > 
> > b) Declare the function completely, and protect all calls to it:
> > 
> > blah.hpp:
> >   void my_jfr_event_handler();
> > 
> > blah.cpp:
> >   void my_jfr_event_handler() {
> > #if INCLUDE_JFR
> >     ...
> > #endif
> >   }
> > 
> > use.cpp:
> >   m() {
> >     JFR_ONLY(my_jfr_event_handler();)
> >   }
> > 
> > I prefer (a), because it looks more straight-forward and cleanly
> > optimizeable. With new set_state
> > function, both options are no longer viable: there is a code that
> > should execute regardless of JFR.
> > 
> > > Please give this a review when you have the chance, I'll check to
> > make sure it causes no issues
> > > configuring without JFR.
> > 
> > I would, but the patch probably got stripped from the mailing list?
> > 
> > -Aleksey
> >