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

[Vladimir Kozlov]

I will push this changes.

Thanks,
Vladimir

On 11/27/12 16:50, Christian Thalinger wrote:
>
> On Nov 27, 2012, at 4:04 PM, Bharadwaj Yadavalli<bharadwaj.yadavalli at oracle.com>  wrote:
>
>>
>> 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.
>
> src/share/vm/opto/parse1.cpp:
>
> +  if (log)  log->done("parse nodes='%d'  live='%d' memory='%d'",
>
> There are two spaces before live (I know it doesn't matter but it's a waste of disk space ;-).
>
> Otherwise this looks good.
>
> -- Chris
>
>>
>> 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.
>>>>>>>>>
>