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 at oracle.com
Thu Jan 14 21:04:31 UTC 2016
Please initialize these in the constructor
112 HeapWord* _last_query_beg;
113 oop _last_query_obj;
114 size_t _last_query_ret;
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);
On 01/14/2016 01:34 AM, Mikael Gerdin wrote:
> cc:ing Jon
> Jon, can you have a look at the logic in the live_words_in_range
> optimization performed here? I don't feel that I have enough knowledge
> about parallel compact to say if it's correct or not.
> On 2016-01-13 16:51, Thomas Schatzl wrote:
>> Hi Tianyang,
>> On Sun, 2016-01-10 at 22:16 +0800, ray alex wrote:
>>> By following the conventions, I remove the prefix "get_" and rename
>>> getter/setter to "last_query_*" in this patch version 4 (attached).
>>> Besides, I ran the measurements mentioned before (both for version 3
>>> and recheck this version 4 ), and there was no regressions.
>>> May I have your reviews on this patch?
>> first, sorry for the delay, holidays and such...
>> I created a CR for this enhancement at
>> I also made some webrevs for this change that fix some minor issues:
>> base patch that includes some compilation fixes:
>> - recent changes introduced some merge errors in psParallelCompact,
>> mostly due to Unified logging
>> - the assert in psParallelCompact.cpp:3137 missed a parameter so the
>> sources did not compile in any debug mode.
>> Some minor style fixes:
>> http://cr.openjdk.java.net/~tschatzl/8146987/webrev.1/ (full)
>> http://cr.openjdk.java.net/~tschatzl/8146987/webrev.0_to_1/ (diff)
> 2152 PSParallelCompact::AdjustPointerClosure closure(cm);
> 2153 PSParallelCompact::AdjustKlassClosure kclosure(cm);
> Please name these oop_closure and klass_closure.
> Otherwise I think this looks good (modulo getting feedback from Jon)
>> - made a few of the new methods private
>> - some coding style fixes: indentation, trailing white spaces,
>> copyright dates
>> - removed the new parMarkBitMap.inline.hpp again because its methods
>> were only called from parMarkBitmap.cpp, fixed includes to
>> - prepare some push message, please check
>> I did run internal gc testing on the .0 webrev (vm.gc testlist), and
>> did some performance verification (mainly on specjbb201x as this was
>> what was on hand). It does (as expected) improve the perf of full gc,
>> although the improvements are not as high as mentioned in your
>> initial email, around 15% average full gc time (for some rather
>> specific configuration). That is still quite good imo.
>> Reproducing the results has been one reason why this reply took a
>> while, it needs a bit of carefully setting up tests to get results
>> that are not drowned in noise due to gc occurring on widely different
>> heap contents, and particularly ergonomics.
>> Not sure how you managed to do this with something like dacapo.
>> SPECjvm2008 in "realistic" configurations does not do a lot of (if
>> any) full gcs.
>> The change looks good to me now, but I need to do some more testing
>> again with the .1 version, to make sure the cleanup did not introduce
>> any regressions anywhere.
More information about the hotspot-gc-dev