Incorrect obj_is_alive logic? (Was: What does BufferBlob::Interpreter in a JVM crash log mean)

[Srinivas Ramakrishna]

I looked a the code and it seems as though obj_is_alive() started out as a
purely debugging/verification
method and subsequently as a result of an earlier JMap related bug fix,
started being used for this other purpose.
We realized at some subsequent point that this would miss some objects in
perm gen, especially in as
much as the identification by JMap was concerned and the comments in
obj_is_alive() state as much.

I think though that unfortunately when there is ambiguity about a perm gen
object's
liveness, we cannot safely return the answer that it is live because, if it
is not, then it may be
holding references to other potentially dead objects that the closure might
want to mess with. I am not sure what the
right fix here is unless we have some means of restricting what the closure
passed to safe_object_iterate()
does. In general, it would be dangerous to declare that an arbitrary perm
gen object is live without being
certain of it (when class unloading is not enabled), but it may be safe to
do so under restricted circumstances
-- circumstances perhaps that hold in the way jmap makes use of
safe_object_iterate().

It would seem to me almost as though you want two versions of
obj_is_alive() -- (as I indicated below) --
the more conservative one (as is currently the case) for the heap
verification, and a second version
which corresponds to Kris' suggestion -- for the very restricted case of
how the heap dump uses
safe_object_iterate() -- for use by safe_object_iterate().

Then we will need to make sure to test both verification with CMS perm gen
sweeping disabled
as well as Jmap.

The other alternative is to bite the bullet re the performance impact of
doing so,
and enable CMS perm gen sweeping / class unloading always, so we can
simplify
safe_object_iterate() quite a bit (the only issue to deal with then is for
the case when
a sweep is in progress when a heap dump wants to occur and we just use the
live map in that case).

Or, may be one wants to let this Jmap anomaly remain and chalk it up as a
shortcoming
of jmap when applied to CMS heaps in that they under-report the objects in
the perm gen.

Sorry that i could not be of more help, but may be my ramblings will help
inform folks of
what may be a proper course here (subject to relative costs and benefits).

thanks!
-- ramki

On Wed, Nov 16, 2011 at 9:57 AM, Srinivas Ramakrishna <ysr1729 at gmail.com>wrote:

> It depends on where this is being used. I haven't looked at the code, but
> if it was being used for verification,
> the old answer would be considered more conservative. If on the other had
> for the identification of object
> liveness then the new answer would be more correct. I'll try and take a
> look at it later today and get
> back with comments.
>
> thanks!
> -- ramki
>
>
> On Wed, Nov 16, 2011 at 4:55 AM, Krystal Mok <rednaxelafx at gmail.com>wrote:
>
>> Hi all,
>>
>> Comments on bug 7066603, inline below:
>>
>> On Wed, Nov 16, 2011 at 4:05 AM, Tom Rodriguez <tom.rodriguez at oracle.com>
>>  wrote:
>>
>>>
>>> On Nov 14, 2011, at 2:00 PM, Martin Hare Robertson wrote:
>>> > The biggest unresolved problem was reported under bug 7066603. Sadly
>>> this seems to have been marked as low priority.
>>>
>>> It seems to have gotten stuck over in the wrong category.  I've moved
>>> into the proper category and updated it.  It appears CMS isn't visiting
>>> some regular Java objects held in perm so they are missing from the dump.
>>>  The recent changes to move Strings and Classes out of perm make this
>>> problem disappear in JDK7.  So it only affects JDK6 at this point.  I
>>> believe the bug is in the obj_is_alive logic that CMS uses to visit only
>>> live objects in perm.
>>>
>>
>> Looking at the repro example in bug 7066603, I agree that the problem
>> seem to lie in obj_is_alive logic.
>>
>> A quick patch to HotSpot 20.0 [1]:
>>
>> diff -r f0f676c5a2c6
>> src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp
>> ---
>> a/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp
>>   Tue Mar 15 19:30:16 2011 -0700
>> +++
>> b/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp
>>   Wed Nov 16 20:38:29 2011 +0800
>> @@ -1143,7 +1143,7 @@
>>          return (dead_map->sizeInBits() == 0) // bit_map has been
>> allocated
>>                 || !dead_map->par_isMarked((HeapWord*) p);
>>        } else {
>> -        return false; // We can't say for sure if it's live, so we say
>> that it's dead.
>> +        return true; // We can't say for sure if it's live, so we say
>> that it's alive.
>>        }
>>      }
>>    }
>>
>> With this patch, re-run the repro, and jhat doesn't complain any more.
>>
>> I'm not sure yet if this is the right change. It's interesting to find
>> that in the repro example, all objects missing from the heap dump seem to
>> be Strings referenced by objects in PermGen; the roots are most probably
>> static fields.
>>
>> Regards,
>> Kris Mok
>>
>> [1]:
>> http://hg.openjdk.java.net/hsx/hsx20/master/file/f0f676c5a2c6/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp
>>
>>
>>> tom
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20111120/aba26147/attachment.htm>