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

[Bharadwaj Yadavalli]

Updated the webrev

http://cr.openjdk.java.net/~bharadwaj/7092905/webrev_03/

Added the ability in LogCompilation tool to print out live nodes info 
from log file.

Please review.

Thanks,

Bharadwaj

On 11/23/2012 9:19 AM, Vladimir Ivanov wrote:
>>> if (_log != NULL) {
>>> - _log->done("phase nodes='%d'", C->unique());
>>> + _log->done("phase name='%s'", _phase_name);
>>> }
>>> Phase name is redundant here - it duplicates info from the head. But I
>>
>> It is not redundant with nested phases and big methods you don't see the
>> head so it is nice to see which phase ends.
> Valid point. I'm not against this change, just looking at it from 
> automated parsing perspective. Current format isn't human-friendly 
> anyway :-)
>
> Best regards,
> Vladimir Ivanov
>
>>> would leave nodes count as is and add live nodes count.
>>
>> Agree. It could be duplicated in the head of the following phase but
>> more info is good.
>>
>> Vladimir K.
>>
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>> On 11/20/12 10:39 PM, Bharadwaj Yadavalli wrote:
>>>>
>>>> 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.
>>>>>>>