Request for Reviews(M): 7092905: C2: Keep track of the number of dead nodes

Bharadwaj Yadavalli bharadwaj.yadavalli at oracle.com
Tue Nov 20 11:39:21 PST 2012


Please review the updated webrev at 
http://cr.openjdk.java.net/~bharadwaj/7092905/webrev_03/

Hopefully I have addressed all the review comments/suggestions.

Thanks,

Bharadwaj

On 11/9/2012 12:35 PM, Vladimir Kozlov wrote:
> Bharadwaj,
>
> Nils just reminded me (for his changes) that you need to fix our 
> LogCompilation parsing tool to recognize new "unique='n' 
> live_tracked='n'" values (old was "nodes='n'"):
>
> src/share/tools/LogCompilation/
>
> Also live_tracked could be simple "live".
>
> Thanks,
> Vladimir
>
> Vladimir Kozlov wrote:
>> VerifyIdealNodeCount flag should be also notproduct since it is used 
>> only in debug VM.
>>
>> The next assert is not needed because it duplicates assert in 
>> live_nodes():
>>
>>     bool check_node_count(uint margin, const char* reason) {
>>       if (unique() + margin > (uint)MaxNodeLimit) {
>> +     assert(dead_node_count() <= unique(), "number of dead nodes 
>> more than created");
>>
>> Also don't split the assert in live_nodes() into 2 lines and maybe 
>> use err_msg_res() to print _unique and _dead_node_count values. Also 
>> I think it should return unsigned int type value as unique() method.
>>
>> Second Compile() constructor does not set _dead_node_count(0).
>>
>> count_live_nodes_by_graph_walk() and NotANode() methods are now used 
>> only in debug code so put them under #ifdef ASSERT.
>>
>>
>> print_missing_nodes() should call count_live_nodes_by_graph_walk() 
>> instead of duplicating code. And it should print to _log itself. 
>> TracePhase() code will have only call to it:
>>
>> 3133   if (VerifyIdealNodeCount) {
>> 3134     Compile::current()->print_missing_nodes();
>> 3135   }
>>
>> And do not check VerifyIdealNodeCount in print_missing_nodes() to 
>> allow call it in debugger without VerifyIdealNodeCount set.
>>
>> I think VerifyIdealNodeCount code should be in ~TracePhase() (end of 
>> phase).
>>
>> Thanks,
>> Vladimir
>>
>> Bharadwaj Yadavalli wrote:
>>> Addressed comments on my previous RFR. Changes at
>>> http://cr.openjdk.java.net/~bharadwaj/7092905/webrev_02/
>>>
>>> These changes are made to keep an (almost) accurate running count of 
>>> the reachable (live) flow graph nodes. This will result in a more 
>>> realistic node count for various phases of C2 to decide on whether 
>>> to proceed with optimizations or not. Prior to these changes, C2 
>>> bails out of compilation based on the number of nodes created which 
>>> typically larger than the number of reachable (live) nodes.
>>>


More information about the hotspot-compiler-dev mailing list