JDK 10 RFR JDK-8167425: Redundant code in method PerfLongVariant::sample

[Shafi Ahmad]

Hi,

Please find updated webrev link.
http://cr.openjdk.java.net/~shshahma/8167425/webrev.01/

Regards,
Shafi

> -----Original Message-----
> From: David Holmes
> Sent: Thursday, March 09, 2017 7:35 AM
> To: Coleen Phillimore <coleen.phillimore at oracle.com>; hotspot-
> dev at openjdk.java.net
> Subject: Re: JDK 10 RFR JDK-8167425: Redundant code in method
> PerfLongVariant::sample
> 
> Hi Coleen,
> 
> On 9/03/2017 1:42 AM, coleen.phillimore at oracle.com wrote:
> >
> > This doesn't look right.
> >
> > Don't you need to still check for _sampled != NULL?
> 
> That use of _sampled was in dead code that is now removed.
> 
>      if (_sample_helper == NULL) return;
> -   if (_sample_helper != NULL) {
>        *(jlong*)_valuep = _sample_helper->take_sample();
> -   }
> -   else if (_sampled != NULL) {
> -     *(jlong*)_valuep = *_sampled;
> -   }
>    }
> 
> This change looks fine to me.
> 
> > Can you remove the JJJ in the comment?
> 
> I second that :)
> 
> Thanks,
> David
> -----
> 
> >  I don't know what tests were run
> > where Jon (aka JJJ in the code) found this, do you?
> >
> > Thanks,
> > Coleen
> >
> >
> > On 3/8/17 10:21 AM, Shafi Ahmad wrote:
> >> Hi,
> >>
> >> May I get the review done for this.
> >>
> >> Regards,
> >> Shafi
> >>
> >>> -----Original Message-----
> >>> From: Shafi Ahmad
> >>> Sent: Wednesday, March 01, 2017 4:27 PM
> >>> To: hotspot-dev at openjdk.java.net
> >>> Subject: FW: JDK 10 RFR JDK-8167425: Redundant code in method
> >>> PerfLongVariant::sample
> >>>
> >>> Hi,
> >>>
> >>> Summary:
> >>> It's a very small change to a single file.
> >>>
> >>> void PerfLongVariant::sample() {
> >>>
> >>> -  assert(_sample_helper != NULL || _sampled != NULL, "unexpected
> >>> state");
> >>> +  // JJJ - This should not happen.  Maybe the first sample is taken
> >>> + // while the _sample_helper is being null'ed out.
> >>> +  // assert(_sample_helper != NULL || _sampled != NULL, "unexpected
> >>> + state"); if (_sample_helper == NULL) return;
> >>>
> >>>    if (_sample_helper != NULL) {
> >>>      *(jlong*)_valuep = _sample_helper->take_sample();
> >>>    }
> >>>    else if (_sampled != NULL) {
> >>>      *(jlong*)_valuep = *_sampled;
> >>>    }
> >>> }
> >>> Above five lines are modified in changeset
> >>>
> http://hg.openjdk.java.net/jdk8/jdk8/hotspot/rev/da91efe96a93#l802.1
> >>> 2 Due to the addition of NULL check and return
> >>>    if (_sample_helper == NULL) return; the else-if block becomes
> >>> redundant and statement 'if (_sample_helper != NULL)' is not needed.
> >>>
> >>> Webrev link:
> http://cr.openjdk.java.net/~shshahma/8167425/webrev.00/
> >>> Jdk10 bug link: https://bugs.openjdk.java.net/browse/JDK-8167425
> >>>
> >>> Testing: jprt.
> >>>
> >>> Thanks,
> >>> Shafi
> >