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

[Vladimir Kozlov]

I forgot to tell that VerifyIdealNodeCount flag should be defined in 
c2_globals.hpp since it is C2 specific flag.

Also how you handle Node::destruct() mathod which may allow to reuse _idx?

The rest comments are inlined below.

Bharadwaj Yadavalli wrote:
> 
> Thanks for your reviews, Vladimir and Christian.
> 
> On Oct 29, 2012, at 10:29 AM, Vladimir Kozlov 
> <vladimir.kozlov at oracle.com> wrote:
>>> Bharadwaj,
>>>
>>> These changes look much better. Sorry, I suggested to add 
>>> verify_live_node_count() method before but your current code don't 
>>> need it, just use count_live_nodes_by_graph_walk().
> 
> OK. Made the necessary changes.
> 
>>> An other note: without LogCompilation verification code should 
>>> produce some info on tty.
> 
> OK. We do not need to print the information about the number of unique 
> nodes created, those that are live and those that are dead to the tty, 
> correct? (Ref. around line 777 and line 816 in compile.cpp)

It is useful information when you try to diagnose node limit bailouts. You can 
add new debug flag PrintIdealNodeCount to output this info to tty.

> 
> That leaves us with the message when there is a mismatch between the 
> live node count arrived at by flow-graph walk and that kept track of via 
> deal_node_list.
> 
> Would you suggest that I print out to tty indicating a mismatch? (Ref. 
> around line 819 in compile.cpp) A mismatch may not always indicate that 
> something is indeed wrong - given that some constant nodes that become 
> unreachable (dead) can later become reachable, for example, due to 
> final_graph_reshaping_walk(). Hence I do not count constant nodes that 
> become unreachable as dead. So printing out a message on the tty might 
> trigger false alarms. So, I chose to print a message in the log file only.

-XX:+VerifyIdealNodeCount is our internal flag and we would expect these 
messages to be not accurate.

> 
> Please let me know if you still think that I should print this info (the 
> one around line 819 in compile.cpp) to tty.

Yes, also you need to dump nodes when output is tty instead of printing their idx:

  useful.at(i)->dump();

Thanks,
Vladimir

> 
> On 10/29/2012 1:56 PM, Christian Thalinger wrote:
>> We still need to adjust MaxNodeLimit.  Maybe some JRuby or Nashorn 
>> benchmarks can help in getting really big methods.
> 
> I agree. However, I felt that arriving at the MaxNodeLimit might need 
> some experimentation with different applications whose method 
> compilations result in creation of graphs with large number of nodes. I 
> do not (yet) know how the current value was arrived at. It appears to me 
> that leaving the current value unchanged in light of these proposed 
> changes would not adversely affect normal applications. So, I thought we 
> could check in these changes and later check in a change to MaxNodeLimit 
> based on a detailed experimentation. What do you think?
> 
>> src/share/vm/opto/compile.cpp:
>>
>> +     _log->elem("node count before optimize compile_id='%d' 
>> unique='%d' live (tracked)='%d' live(graph_walk)='%d'",
>>
>> I think the log element name needs to have _'s so we can match it (if 
>> we want to):
>>
>> +     _log->elem("node_count_before_optimize compile_id='%d' 
>> unique='%d' live (tracked)='%d' live(graph_walk)='%d'",
> 
> OK. Changed.
> 
> Thanks,
> 
> Bharadwaj
>