RFR: 8150720: Cleanup code around PrintOptoStatistics

[Aleksey Shipilev]

On 02/26/2016 03:22 PM, Claes Redestad wrote:
> Hi,
> 
> On 2016-02-26 12:57, Aleksey Shipilev wrote:
>> On 02/26/2016 02:40 PM, Claes Redestad wrote:
>>> webrev: http://cr.openjdk.java.net/~redestad/8150720/webrev.00/
>> Wait, I don't get these parts in the original code:
>>
>> src/share/vm/opto/node.cpp:
>>   621 #ifdef ASSERT
>>   622     if( edge_end+node_size == compile->node_arena()->hwm() ) {
>>   623       reclaim_in  += edge_size;
>>   624       reclaim_node+= node_size;
>>   625     }
>>   626 #else
>>   627     // It was; free the input array and object all in one hit
>>   628     compile->node_arena()->Afree(_in,edge_size+node_size);
>>   629 #endif
>>
>> ...
>>
>>   639     // Free just the object
>>   640 #ifdef ASSERT
>>   641     if( ((char*)this) + node_size == compile->node_arena()->hwm() )
>>   642       reclaim_node+= node_size;
>>   643 #else
>>   644     compile->node_arena()->Afree(this,node_size);
>>   645 #endif
>>
>> We are not freeing when ASSERT is not defined? O_o The patch that
>> apparently does an equivalent transformation of the original code makes
>> it even creepier.
> 
> yes, this is a weirdness in the original code, and not preserving this
> pattern makes the VM crash early in fastdebug mode.

Dear God.

> There is also a small "leak" in the corner case where "edge_end ==
> (char*)this" since we're not freeing the input array in this case.
> 
> Either way I think diving into this is out of scope for this cleanup,
> though.

Yes, I agree. I don't see other troubles in the patch.

-Aleksey


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160226/0d161795/signature.asc>