RFR (S): 8043243: convert SCAN_AND_FORWARD, SCAN_AND_ADJUST_POINTERS, SCAN_AND_COMPACT macros to methods

[Marcus Larsson]

Hi Stefan,

On 08/28/2014 02:14 PM, Stefan Karlsson wrote:
> Hi Marcus,
>
> On 28/08/14 13:46, Marcus Larsson wrote:
>> Hi,
>>
>> I would like reviews for the following patch converting the 
>> SCAN_AND_* macros into (inline) methods.
>>
>> Short summary:
>> The change is based on the demacroify.patch (bug attachment), using a 
>> proxy class to enable different functions for obj_size and similar to 
>> be used for different types of Spaces. Each type of space has a proxy 
>> defining which methods should be used in the scan_and_* functions.
>> Made no changes to the actual code in the macros, except for 
>> replacing a multi-line debug_only(...) with #ifdef ASSERT ... #endif.
>
> Some background to this change. I wrote it as PoC to see if we could 
> get rid of these large macros. At that time there werw concerns raised 
> that some compilers might not be able to inline the template 
> functions. I checked the code generated by gcc and it was inlined as 
> expected, but I didn't check the code from the other compilers.
>
> Are the list of benchmarks below performance runs? Was it done on 
> different platforms? Did you measure the changed phases 
> (prepare_for_compation, adjust_pointers, etc.) or did you measure the 
> benchmark score?

The measurements were performance runs, but I only measured benchmark 
score. I reran the benchmarks, this time to also verify/look at overall 
GC times and the different phase times. I could find no significant 
changes in these times on any of the tested platforms (Solaris, Linux, 
Windows).

>
>>
>> Webrev:
>> http://cr.openjdk.java.net/~brutisso/webrev-8043243/
>
> Looking at the patch now, I wonder if we shouldn't get rid of the 
> instance variables in the *SpaceProxy classes and instead pass down 
> 'this' from the call sites? And make these classes AllStatic.

Made the proxies AllStatic (reruns described above used this new version).
Also followed Mikael's suggestion and made explicit calls to the known 
member functions in proxies for CompactibleFreeListSpace and 
G1OffsetTableContigSpace.

New webrev:
http://cr.openjdk.java.net/~brutisso/webrev-8043243v2/

Thanks,
Marcus

>
> thanks,
> StefanK
>
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8043243
>>
>> Testing:
>> jprt without problems
>> SPECjbb2013, SPECjbb2005, SPECjvm2008 - no significant changes
>>
>> Thanks,
>> Marcus
>