RFR (S): 8027295: Free CSet takes ~50% of young pause time
stefan.karlsson at oracle.com
Wed Feb 26 01:38:27 PST 2014
On 2014-02-25 14:51, Thomas Schatzl wrote:
> Hi Stefan,
> On Mon, 2014-02-24 at 14:15 +0100, Stefan Karlsson wrote:
>> Hi Thomas,
>> Not a full review, but a comment below:
>> On 2014-02-24 12:40, Thomas Schatzl wrote:
>>> Hi all,
>>> can I have reviews for the remaining parts of 8027295 after splitting
>>> out code cache remembered set data structure changes (JDK-8025406)?
>>> It only includes several smaller changes around CSet freeing that
>>> improve performance.
>>> Note that this change is based on 8035406 also out for review
>>> - fast card cache changes
>>> - pad FCC rows to cache line size to avoid any false sharing (every
>>> row represents the card cache for a single worker thread)
>> Is it really enough to just pad the size and not the start address of
> Fixed in
The padding part of this change looks good, consider it reviewed if you
push that as a separate RFE.
I have a couple of style comments:
+ // Pad rows to cache line sizes.
+ static const int ints_per_cache_line = DEFAULT_CACHE_LINE_SIZE / sizeof(int);
int n_par_rs = HeapRegionRemSet::num_par_rem_sets();
+ _from_card_cache_max_regions = align_size_up(max_regions, ints_per_cache_line);
+ // Need to make sure that the arrays are aligned too. Allocate a suitably
+ // large array to be able to align its base address later. We do not store
+ // the old pointer as we never free that memory.
+ size_t num_ints_to_alloc = n_par_rs * _from_card_cache_max_regions + ints_per_cache_line - 1;
+ int* data = NEW_C_HEAP_ARRAY(int, num_ints_to_alloc, mtGC);
+ data = (int*) (align_size_up_((uintptr_t)data, DEFAULT_CACHE_LINE_SIZE));
Could you move the "int n_par_rs = " line down. It's sort of hidden
between the setup of ints_per_cache_line and the first usage of
Could you add a comment that you do this padding to avoid false sharing?
It would have been nice to be able to use the code in padded.hpp. See
PaddedArray::create_unfreeable(). Do you think it would be worth
extracting the padding code to a new class for arrays of arrays with
More information about the hotspot-gc-dev