code review for JVM/TI DynamicCodeGenerated event fix (7039447)

[David Holmes]

Hi Dan,

Daniel D. Daugherty said the following on 04/28/11 13:56:
> David, thanks for the review.
> 
> Dmitry and I have been chatting on the side about various
> null name handling styles. Feel free to chime in...

Ok :) I was thinking simply do:

  event._event_data.dynamic_code_generated.name =
            os::strdup(name != NULL ? name : "unknown_code");

and then all you need is an unconditional free:

!     case TYPE_DYNAMIC_CODE_GENERATED: {
         JvmtiExport::post_dynamic_code_generated_internal(
           _event_data.dynamic_code_generated.name,
           _event_data.dynamic_code_generated.code_begin,
           _event_data.dynamic_code_generated.code_end);
+         os::free(_event_data.dynamic_code_generated.name);
         break;
+     }

Cheers,
David


> Dan
> 
> 
> On 4/27/2011 5:11 PM, David Holmes wrote:
>> Hi Dan,
>>
>> I was going to make a comment about the handling of a null name, but 
>> you have two thumbs up already and it really is just a style thing so ...
>>
>> Looks good to me. :)
>>
>> Cheers,
>> David
>>
>>
>> Daniel D. Daugherty said the following on 04/28/11 05:46:
>>> Greetings,
>>>
>>> I have a fix for a bug in JVM/TI DynamicCodeGenerated event posting:
>>>
>>>    7039447 2/1 java profiling is broken in build 139 (garbage in
>>>                function name)
>>>
>>> The bug has actually been tracked back to HSX-21-B04/JDK7-B134, but
>>> the failure mode can be intermittent. Here is the webrev URL:
>>>
>>>    http://cr.openjdk.java.net/~dcubed/7039447-webrev/0/
>>>
>>> Here is my proposed commit message:
>>>
>>> 7039447: 2/1 java profiling is broken in build 139 (garbage in 
>>> function name)
>>> Summary: The name in a deferred JVM/TI DynamicCodeGenerated event 
>>> needs to be explicitly saved.
>>> Reviewed-by:
>>>
>>>
>>> I'm targeting this fix at HSX-21-B12/JDK7-B142; I just missed the
>>> RT_Baseline cutoff for HSX-21-B11/JDK7-B141. Since we are getting
>>> down to the wire on JDK7, I would like at least three reviewers.
>>>
>>> I'm in the process of running the following test suites (the
>>> JPDA parts of the Serviceability stack):
>>>
>>>    nsk.jvmti, nsk.jvmti_unit,
>>>    nsk.jdwp,
>>>    nsk.jdi, SDK_JDI, SDK_JLI,
>>>    nsk.hprof, nsk.jdb,
>>>    nsk.sajdi,
>>>    vm.heapdump, SDK_MISC_ATTACH, SDK_MISC_JVMSTAT, SDK_MISC_TOOLS
>>>
>>> on the following configs:
>>>
>>>    {Solaris X86, WinXP} x {Client VM} x {product, fastdebug} x 
>>> {Xmixed, Xcomp}
>>>
>>> So far preliminary results show no regressions and no new failures.
>>>
>>> I've also provided JPRT test bits to the Analyzer team and they
>>> have confirmed that the issue is resolved.
>>>
>>> Thanks, in advance, for any reviews.
>>>
>>> Dan
>>>
>>>