JDK 10 RFR JDK-8167425: Redundant code in method PerfLongVariant::sample (original) (raw)

David Holmes david.holmes at oracle.com
Thu Mar 9 02:05:14 UTC 2017

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;

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(samplehelper != NULL || sampled != NULL, "unexpected state"); + // JJJ - This should not happen. Maybe the first sample is taken // + while the samplehelper is being null'ed out. + // assert(samplehelper != NULL || sampled != NULL, "unexpected + state"); if (samplehelper == NULL) return; if (samplehelper != NULL) { (jlong)valuep = samplehelper->takesample(); } 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 (samplehelper == NULL) return; the else-if block becomes redundant and statement 'if (samplehelper != 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