[9] RFR (M): 8011858: Use Compile::live_nodes() instead of Compile::unique() in appropriate places

[Vladimir Ivanov]

Looks good.
I'll sponsor the change.

Best regards,
Vladimir Ivanov

On 7/18/15 4:24 AM, Vladimir Kozlov wrote:
> Thank you, Vlad
>
> It looks good. We usually don't put bug id into comments. So your
> previous version on cr.openjdk is fine.
>
> Second reviewer should look on and sponsor it with you listed as
> contributor (I see you signed OCA already).
>
> Thanks,
> Vladimir
>
> On 7/17/15 3:47 PM, Vlad Ureche wrote:
>> Hi,
>>
>> Please review the following patch for JDK-8011858. Big thanks to
>> Vladimir Kozlov for his patient guidance while working on this!
>>
>> *Bug:* https://bugs.openjdk.java.net/browse/JDK-8011858
>>
>> *Problem:* Throughout C2, local stacks are used to prevent recursive
>> calls from blowing up the system stack. These are sized based on the
>> total number of nodes in the compilation run (e.g. C->unique()).
>> Instead, they should be sized based on the live node count
>> (C->live_nodes()).
>>
>> Now, with the increased difference between live_nodes (limited at
>> LiveNodeCountInliningCutoff, set to 40K) and unique nodes (which can go
>> up to 240K), it is important to not over-estimate the size of stacks.
>>
>> *Solution:* This patch mirrors a patch written by Vladimir Kozlov for
>> JDK8u. It replaces the initial sizes from C->unique() to
>> C->live_nodes(), preserving any shifts (divisions) and offsets. For
>> example, in the compile.cpp patch
>> <http://cr.openjdk.java.net/~kvn/8011858/webrev/src/share/vm/opto/compile.cpp.patch>:
>>
>>
>> |-  Node_Stack nstack(unique() >> 1);
>> +  Node_Stack nstack(live_nodes() >> 1);
>> |
>>
>> There is an issue described at
>> https://bugs.openjdk.java.net/browse/JDK-8131702 where I took the
>> workaround from Vladimir’s patch.
>>
>> *Webrev:* http://cr.openjdk.java.net/~kvn/8011858/webrev/ or
>> http://vladureche.ro/webrev/8011858
>> <http://vladureche.ro/webrev/8011858>(updated, includes a link to bug
>> 8121702)
>>
>> *Tests:* Running jtreg with the compiler, runtime and gc tests on the
>> dev <http://hg.openjdk.java.net/jdk9/dev> branch shows the same status
>> before and after the patch: 808 tests passed, 16 failed and 6 errors
>> <http://vladureche.ro/webrev/8011858/JTreport/html/index.html>. What
>> would be a stable point where all tests are expected to pass, so I can
>> test the patch there? Maybe jdk9 <http://hg.openjdk.java.net/jdk9/jdk9>?
>>
>> Thanks,
>> Vlad
>>