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

[Jon Masamitsu]

http://cr.openjdk.java.net/~sjohanss/8062063/hotspot.02/src/share/vm/gc_implementation/g1/g1PageBasedVirtualSpace.cpp.frames.html

Can this be put under the *else *branch for the *if (_special)*

  147   if (AlwaysPreTouch) {
  148     os::pretouch_memory(page_start(start), page_start(end));
  149   }

The BitMap

   52   // Bitmap used to keep track of which pages are dirty or not for _special
   53   // spaces. This is needed because for those spaces the underlying memory
   54   // will only be zero filled the first time it is committed. Calls to commit
   55   // will use this bitmap and return whether or not the memory is zero filled.
   56   BitMap _dirty;

is used to decide what parts of the memory being brought back (the 
equivalent of
the commit() for _special) to zero.  The commit() is passed a start 
address and size.  What
is the situation where you need the information in _dirty?  Meaning you 
could zero
the range passed into commit() i.e.,  (start, start+size), yes? When 
would that differ
from using _dirty?

Jon

On 1/9/2015 6:31 AM, Stefan Johansson wrote:
> 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
>>>
>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150109/904551af/attachment.htm>