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

[Stefan Johansson]

Updated links:
http://cr.openjdk.java.net/~sjohanss/8062063/hotspot.01-02/
http://cr.openjdk.java.net/~sjohanss/8062063/hotspot.02/

Cheers,
Stefan

On 2015-01-09 12:31, Stefan Johansson wrote:
> Hi Thomas and Kim,
>
> On 2015-01-08 23:02, Thomas Schatzl wrote:
>> Hi Stefan,
>>
>> On Thu, 2015-01-08 at 14:05 -0500, Kim Barrett wrote:
>>> On Jan 8, 2015, at 12:59 PM, Stefan 
>>> Johahttp://cr.openjdk.java.net/~sjohanss/8062063/hotspot.01-02/nsson 
>>> <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 somewhat tend to agree with Kim here.
>>
>> Maybe one of _needs_clear_on_commit, _not_zero_on_commit,
>> _is_clear_on_commit, or _pages_dirty_after_uncommit, but ymmv. :)
> I too agree, but I changed it to simply _dirty and added a big comment 
> for it. This lets me keep the current logic where a 1 in the bitmap 
> means it is not filled with zeros (dirty).
>
> New webrev and incremental one:
> http://wikifiles.se.oracle.com/dev/sjohanss/8062063/hotspot.02/
> http://wikifiles.se.oracle.com/dev/sjohanss/8062063/hotspot.01-02/
>
> Thanks,
> Stefan
>
>>> 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.
>>>
>>> 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.
>> It simply assumes that a commit() zero-fills the page lazily.
>>
>> I do not think it is worth worrying a lot about it. There need to be a
>> lot of circumstances involved, and the new change at least always 
>> avoids the
>> clearing of the Java heap space.
>>
>> The best solution would simply be doing away with the pre-commit hack
>> when using large pages :)
>>
>>    Thomas
>>
>>
>>
>