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

[Vladimir Kozlov]

I pushed changes:

http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/af60f1cb36f2

Thanks,
Vladimir K

On 7/30/15 2:33 AM, Vladimir Ivanov wrote:
> 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
>>>