Request for reviews (M): 7059037: Use BIS for zeroing on T4

[Ramki Ramakrishna]

On 8/25/2011 4:51 PM, Vladimir Kozlov wrote:
> Ramki,
>
> Ramki Ramakrishna wrote:
>> Hi Vladimir --
>>
>> On 8/24/2011 5:52 PM, Vladimir Kozlov wrote:
>>> http://cr.openjdk.java.net/~kvn/7059037/webrev
>>>
>>> 7059037: Use BIS for zeroing on T4
>>>
>> ...
>>> Zap new object in CollectedHeap::allocate_from_tlab_slow() instead 
>>> of zeroing it
>>> since it will be cleaned later in init_obj().
>
> TLAB::allocate() zaps new objects so I think allocate_from_tlab_slow() 
> should also zap new object (and I copied code from 
> ThreadLocalAllocBuffer::allocate()) instead of cleaning it since it 
> will be cleaned later in init_obj().
>

I see. OK I agree that this is the right thing to do for concurrent 
gc's, although i wish this could be
cleanly abstracted based on the collector, rather than a blanket 
imposition from concurrent gc's.

>>> Fixed call sites of check_for_bad_heap_word_value() where klass is not
>>> initialized to avoid the verification failure.
>>>

I see that the skip_header_HeapWords() used in GCH:: 
check_for_non_bad_heap_word_value() was
not extended to the CH::check_for_bad_heap_word_value().

Your changes look good; thanks for fixing up the shortcomings. I'll 
check with my colleagues on
the need to clean this up (in a separate CR of course) so that the 
concurrent GC'isms do not leak out in
this manner into the general code, or at least are left sufficiently 
abstract when they can be.

-- ramki
>
> % /java/re/jdk/7/latest/binaries/solaris-i586/fastdebug/bin/java 
> -XX:+CheckMemoryInitialization -Xcomp t
> VM option '+CheckMemoryInitialization'
> # To suppress the following error report, specify this argument
> # after -XX: or in .hotspotrc:  SuppressErrorAt=/collectedHeap.cpp:98
> #
> # A fatal error has been detected by the Java Runtime Environment:
> #
> #  Internal Error 
> (/tmp/workspace/jdk7-2-build-solaris-i586-product/jdk7/hotspot/src/share/vm/gc_interface/collectedHeap.cpp:98), 
> pid=27663, tid=2
> #  assert((*(intptr_t*) (addr + slot)) != ((intptr_t) badHeapWordVal)) 
> failed: Found badHeapWordValue in post-allocation check
> #
> # JRE version: 7.0-b147
> # Java VM: Java HotSpot(TM) Server VM (21.0-b17-fastdebug compiled 
> mode solaris-x86 )
>
> Vladimir
>
>>
>> Can you describe why these two changes were necessary? There was 
>> already support
>> for skipping headers for concurrent GC's when zapping and verifying. 
>> Did something
>> change that caused this to be changed.
>>
>> I haven't looked at the rest of the files, but a high level 
>> description of the need to
>> make this change would allow me to review the changes that 
>> necessitated this,
>> and whether it could not be done more easily otherwise (using the 
>> existing
>> framework of skipping a preamble of words in the object).
>>
>> -- ramki