RFR (M): 8146987: Improve Parallel GC Full GC by caching results of live_words_in_range() [Was: Re: [PATCH] enhancement to ParallelScavenge Full GC]

[Jon Masamitsu]

Thomas,

You had mentioned that ParMarkBitMap::live_words_in_range()
could be made more efficient.   Did you have a specific idea there?
Or just horrified :-) by everything that it does?

Jon

On 1/25/2016 3:51 AM, Thomas Schatzl wrote:
> Hi,
>
> ----- jon.masamitsu at oracle.com wrote:
>
>> On 1/15/2016 3:43 AM, Thomas Schatzl wrote:
>>> Hi all,
>>>
>>> On Thu, 2016-01-14 at 13:04 -0800, Jon Masamitsu wrote:
>> http://cr.openjdk.java.net/~tschatzl/8146987/webrev.1/src/share/vm/gc
>>>> /parallel/psCompactionManager.hpp.frames.html
>>>>
>>>> Please initialize these in the constructor
>>>>
>>>> 112 HeapWord* _last_query_beg;
>>>> 113 oop _last_query_obj;
>>>> 114 size_t _last_query_ret;
>>>>
>>>>
>> http://cr.openjdk.java.net/~tschatzl/8146987/webrev.1/src/share/vm/oo
>>>> ps/oop.hpp.udiff.html
>>>>
>>>> Any reason to use "pc" as the parameter name here and
>>>> not "cm" as is used elsewhere?
>>>>
>>>> - void pc_update_contents();
>>>> + void pc_update_contents(ParCompactionManager* pc);
>>>>
>>>     new webrevs (also including Mikael's comments) at
>>>
>>> http://cr.openjdk.java.net/~tschatzl/8146987/webrev.2/ (full)
>>> http://cr.openjdk.java.net/~tschatzl/8146987/webrev.1_to_2/ (diff)
>> Patch looks good.  If you decide to add the comment I suggested
>> (off-line) for  ParMarkBitMap::live_words_in_range_use_cache(), I don't
>> need a new webrev.
> Just fyi, here is the suggested diff for the comment:
>
> --- a/src/share/vm/gc/parallel/parMarkBitMap.cpp	Wed Jan 20 13:15:11 2016 +0100
> +++ b/src/share/vm/gc/parallel/parMarkBitMap.cpp	Mon Jan 25 12:49:08 2016 +0100
> @@ -141,6 +141,8 @@
>       last_ret = last_ret + live_words_in_range_helper((HeapWord*)last_obj, end_obj);
>       last_obj = end_obj;
>     } else if (end_obj < last_obj) {
> +    // The cached value is for an object that is to the left (lower address) of the current
> +    // end_obj. Calculate back from that cached value.
>       if (pointer_delta((HeapWord*)end_obj, (HeapWord*)beg_addr) > pointer_delta((HeapWord*)last_obj, (HeapWord*)end_obj)) {
>         last_ret = last_ret - live_words_in_range_helper((HeapWord*)end_obj, last_obj);
>       } else {
>
> I will push this as soon as the repo is open again.
>
> Thanks everyone,
>    Thomas