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

[coleen.phillimore at oracle.com]

This doesn't look right.

Don't you need to still check for _sampled != NULL?

Can you remove the JJJ in the comment?  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.12
>> 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