[9] RFR(M): 8129847: Compiling methods generated by Nashorn triggers high memory usage in C2

[Zoltán Majó]

P.S.: Forgot to mention:

On 11/26/2015 02:41 PM, Zoltán Majó wrote:
>
> Compile::Compile()
> Compile::inline_boxing_calls()
> Compile::inline_incrementally_one()
> Compile::inline_incrementally()
> Compile::inline_string_calls()
>

At the above code locations, I experimented with performing a 
PhaseRenumberLive in addition to PhaseRemoveUseless.

Thank you and best regards,


Zoltan

>
> (The current version performs renumbering only in Compile::Optimize().)
>
> With that version we get 46% reduction of the reproducer's resident 
> set size (RSS) instead of 39% (the current version). The reduction 
> with Nashorn/Octane differs as well: 8% vs. 5% (current version).
>
> But the overhead due to compilation is higher as well and that results 
> in a performance regression of 3% with SPECjvm2008-MPEG. That is why I 
> decided to go with the current version that does not regress.
>
> So I would like to first get this version in and see if we get 
> problems (bugs or regressions). If all looks fine, we can think of 
> enabling PhaseRenumberLive at other places as well (not necessarily 
> all 6 from above, only those that bring good benefits for a reasonable 
> cost).
>
> Thank you and best regards,
>
>
> Zoltan
>
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8059241
>>
>> On 11/26/15 3:39 PM, Zoltán Majó wrote:
>>> Hi Tobias,
>>>
>>>
>>> On 11/26/2015 09:41 AM, Tobias Hartmann wrote:
>>>> Hi Zoltan,
>>>>
>>>> nice improvements! The implementation looks good to me.
>>>
>>> thank you for your feedback!
>>>
>>>> Here are some questions / suggestions:
>>>> - In the constructor of PhaseRenumberLive you check the value of
>>>> RenumberLiveNodes and pass the corresponding PhaseNumber to
>>>> PhaseRemoveUseless. But shouldn't RenumberLiveNodes always be true
>>>> here? Maybe you should add an assert.
>>>
>>> You are right, RenumberLiveNodes must be always true when performing a
>>> PhaseRenumberLive. I removed the check and added an assert. (I had that
>>> check from an earlier version that, depending on the value of the
>>> RenumberLiveNodes, could perform a use PhaseRenumberLive in place of a
>>> PhaseRemoveUseless.)
>>>
>>>> - The comment says that "PhaseGVN::_nodes" is not updated because it's
>>>> always empty when this phase is used. Should we add an assert here as
>>>> well?
>>>
>>> An assert does not hurt here so I added one that checks if the size of
>>> PhaseGVN::_nodes is 0 when PhaseRenumberLive starts. As a I side 
>>> effect,
>>> I had to change the type of PhaseGVN::_nodes from Node_Array to
>>> Node_List (a subtype of Node_Array that keeps a count of the number of
>>> elements).
>>>
>>> Here is the updated webrev:
>>> [9]: http://cr.openjdk.java.net/~zmajo/8129847-9/webrev.05/
>>> [8u]: http://cr.openjdk.java.net/~zmajo/8129847-8u/webrev.05/
>>>
>>> Testing:
>>> - JPRT;
>>> - locally (linux_x64) executed all hotspot test, all tests pass that
>>> pass with the unmodified build.
>>>
>>> Thank you and best regards,
>>>
>>>
>>> Zoltan
>>>
>>>>
>>>> Best,
>>>> Tobias
>>>>
>>>>
>>>> On 25.11.2015 15:57, Zoltán Majó wrote:
>>>>> Hi,
>>>>>
>>>>>
>>>>> please review the patch for 8129847.
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8129847
>>>>>
>>>>> Problem: JDK-8014959 and JDK-8058148 have increased the value of the
>>>>> LiveNodeCountInliningCutoff and MaxNodeLimit thresholds. The
>>>>> thresholds closely affect the number of unique compiler nodes within
>>>>> compilations. As the size of many of the C2 compiler's data
>>>>> structures is proportional to the number of unique nodes, the changed
>>>>> thresholds increase the memory usage of the C2 compiler. In some
>>>>> cases (e.g., when compiling Nashorn-generated methods), the memory
>>>>> usage increases significantly.
>>>>>
>>>>> Solution: This changeset proposes to add a new compiler phase,
>>>>> PhaseRenumberLive, that (1) identifies live nodes and then (2)
>>>>> assigns a new ID to each node. Each node is assigned a distinct Node
>>>>> IDs from the interval [0, x), where 'x' is the number of live nodes.
>>>>> PhaseRenumberLive updates the compiler's unique count to 'x'.
>>>>> Decreasing the compiler's count of unique nodes reduces the
>>>>> compiler's memory usage.
>>>>>
>>>>> Webrev: I created a webrev for both 8u and 9 because the reproducer
>>>>> attached with the bug report runs only with 8u:
>>>>> [9]: http://cr.openjdk.java.net/~zmajo/8129847-9/webrev.04/
>>>>> [8u]: http://cr.openjdk.java.net/~zmajo/8129847-8u/webrev.04/
>>>>>
>>>>> Testing:
>>>>> - JPRT
>>>>> - Octane/Nashorn
>>>>>
>>>>> Evaluation:
>>>>> - reduction of the number of unique nodes: see following table with
>>>>> top 5 methods in reproducer (methods were ranked in decreasing order
>>>>> of their maximum unique count):
>>>>> http://cr.openjdk.java.net/~zmajo/8129847-8u/node_count.html
>>>>> - reduction of resident set size (RSS) of the VM: 39% with
>>>>> reproducer, 5% on average with Octane/Nashorn;
>>>>> - no performance regression (Octane/Nashorn, SPECjvm2008).
>>>>>
>>>>> Thank you and best regards,
>>>>>
>>>>>
>>>>> Zoltan
>>>>>
>>>
>