RFR: 8233497: Optimize default method generation by data structure reuse

[Claes Redestad]

On 2019-11-15 15:49, Lois Foltan wrote:
> On 11/8/2019 6:57 AM, Claes Redestad wrote:
>> Hi,
>>
>> when loading classes with complex hierarchies and many default methods,
>> we can end up spending significant time in
>> DefaultMethods::generate_default_methods
>>
>> This optimization reduces work done and memory requirements by reusing
>> allocated data structures. For example by maintaining free lists of
>> allocated Node objects.
>>
>> Bug:    https://bugs.openjdk.java.net/browse/JDK-8233497
>> Webrev: http://cr.openjdk.java.net/~redestad/8233497/open.00/
>>
>> Testing: Tier1-3, will make sure tier4-7 pass before push
>>
>> Performance notes: On one of our more complex startup tests we see a 3%
>> improvement on the execution time total. Much less on simpler
>> applications.
>>
>> I've not done a formal complexity analysis, but I think the memory
>> complexity is now down from O(N*M) to O(N+M) where N is the number of
>> classes and interfaces in the hierarchy and M the number of methods of
>> interest in that hierarchy. Algorithmic complexity is probably O(N*M)
>> still, but with much better constants.
>>
>> Special thanks to Lois for patience and persistence over several rounds
>> of pre-review!
>>
>> Thanks!
>>
>> /Claes
> 
> Hi Claes,
> 
> I have reviewed your final webrev and I think this looks good.  It 
> certainly is an area that can benefit from performance improvements so 
> thank you for putting the time in on this. 

Lois, again thank you for reviewing and suggesting a lot of improvements
along the way!

> A couple of final comments:
> 
> - line #79: Can you update the comment, it indicates that new_node_data 
> takes an InstanceKlass* parameter which has been removed
> 
> - line #716: I would like an assert as part of the if statement in the 
> situation where _free_scopes is not empty and the StateRestoreScope node 
> is being popped instead of newly allocated. The assert would then check 
> if that StateRestorerScope's _marks GrowableArray is empty, which it 
> should be.  That would make me more comfortable with the idea that a 
> previous StateRestoreScope's _marks array's data is not getting mixed 
> with new data being established for a new Node in the hierarchy.
> 

Done:

http://cr.openjdk.java.net/~redestad/8233497/open.01/

/Claes