JDK 10 RFR JDK-8167425: Redundant code in method PerfLongVariant::sample
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Thu Mar 9 14:06:53 UTC 2017
On 3/8/17 9:05 PM, David Holmes wrote:
> 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.
I see it all now.
Coleen
>
> 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.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
>>
More information about the hotspot-dev
mailing list