RFR: 8062063: Usage of UseHugeTLBFS, UseLargePagesInMetaspace and huge SurvivorAlignmentInBytes cause crashes in CMBitMapClosure::do_bit

[Stefan Johansson]

Thanks Kim,

On 2015-01-08 20:05, Kim Barrett wrote:
> On Jan 8, 2015, at 12:59 PM, Stefan Johansson <stefan.johansson at oracle.com> wrote:
>> Sorry for doing a re-spin on this, but since this is targeted to go into 8 as well I want to minimize the risk of introducing a regression.
>>
>> After yesterdays comments I started thinking more about what regressions this fix might cause and today I've had good discussions with Thomas and Mikael. I've also did some quick measurements that shows additional time for the YCs expanding the heap after a shrink. Since we don't really need the heap regions to be cleared I think we need to avoid this regression, by going with another solution and I don't think having this time added to the full GC shrinking the heap is wanted either.
>>
>> The first proposal that is explained in the bug-report would avoid clearing memory that don't have to be cleared, but just doing the simple solution explained there might cause startup regressions due to touching memory during startup that isn't needed. Mixing that approach with the one proposed yesterday will allow us to only clear memory when absolutely needed. See new webrev here:
>> http://cr.openjdk.java.net/~sjohanss/8062063/hotspot.01
>>
>> This approach leaves the clearing to the listener registered with each mapper and for the bitmaps this will make sure that they are cleared, but for the heap we won't do anything (because the heap has no requirement of having zeroed backing memory).
> I think I might prefer having the new bitmap called _zero_filled and
> flip the sense of it.  The present name, _needs_zeroing, is mildly
> confusing to me, since whether zeroing is needed is caller-dependent.
I like that name, I hoped someone would come up with a better name, 
didn't fully like _needs_zeroing but wanted to get it out before leaving 
work.
> It seems to me this new version can result in unnecessary page
> clearing; commit will return true if any page in the range is not
> zeroed.  This can lead to a caller that needs zeroed pages clearing
> the entire requested range, even if only some (perhaps small) subset
> of the range is actually dirty.
>
> Of course, the previous attempted fix also had unnecessary page
> clearing, since dirty pages were being cleared even if the caller
> doesn't care.  The new code seems likely to be an improvement overall.
True, we will likely clear more pages than needed but as you say it is 
still an improvement to the other solutions.
> In the context of fixing the bug at hand, I think this change looks
> good, up to the naming and sense of the new bit map.
>
> But it looks like the API provided by G1PageBasedVirtualSpace is less
> than ideal in this area, and could perhaps use further work.  Though
> it might not be worth worrying about, as the cases where it matters
> may be rare and not especially important.
>

Yes, there are still things that could be improved but for this bug fix 
I agree that we should not do to much.

Thanks,
Stefan