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

[Bharadwaj Yadavalli]

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)

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.

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

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