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

[Tobias Hartmann]

Hi Zoltan,

the updated fix looks good to me!

Best,
Tobias

On 26.11.2015 13:39, 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
>>>
>