for review (M): 6863023: need non-perm oops in code cache for JSR 292

Y.S.Ramakrishna at Sun.COM Y.S.Ramakrishna at Sun.COM
Tue Sep 8 15:06:21 PDT 2009


I've started looking at the entire webrev (the
second of the two links you provided below).

I am not done yet, but had two small comments:

(1) the "DetectScavengeRoot" closure appears to check for the
     presence of any non-perm oop, whereas your interest is really
     in something that is in a space subject to a partial collection
     (i.e. a scavenge in our current collectors). So your current
     check is too loose and will often identify nmethods to have
     "scavengable" oops when in fact they do not (their references
     having moved into the old gen which is only collected during
     a full collection). I think you need to change the check:-

1651   virtual void do_oop(oop* p) {
1652     if ((*p) != NULL && !(*p)->is_perm()) {
                               ^^^^^^^^^^^^^^^^
                              !(*p)->is_is_young()) or equivalent

1653       NOT_PRODUCT(maybe_print(p));
1654       _detected_scavenge_root = true;
1655     }
1656   }

(2) Could the removal of nmethods from the scavengable list following
     flush get potentially expensive as you walk down the list to find the
     appropriate nmethod (could the list get long, at least with
     large young gens and with certain kinds of dynamic language applications?).
     Would it be worthwhile to make this scavengable list doubly linked
     so as to allow constant time deletion?
     Depends on how frequently nmethod flushing occurs in the new
     world of scavengable oops in nmethods, i suppose...

Next I'll go and check that the refs from the scavengable
list of nmethods act (in general, unless activations are present),
as weak roots, not strong roots (otherwise they would constitute a
temporary "leak" until the references got promoted to the old generation,
modulo my remark (1) above), but wanted to get these comments to
you first.

later.
-- ramki



On 09/08/09 12:19, Y.S.Ramakrishna at Sun.COM wrote:
> 
> Hi John --
> 
> On 09/08/09 01:38, John Rose wrote:
>> I have sufficient reviews for 6863023 as of webrev.03, but the GC 
>> interactions are (sigh) more subtle than I anticipated, and I want to 
>> get it right.
>>
>> Here is a fresh layer of changes, required to ensure that strong-root 
>> nmethods get scavenged exactly once.
>>   http://cr.openjdk.java.net/~jrose/6863023/diff-02-to-03 (these 
>> specific changes)
> 
> I looked at the above and the changes look fine to me. I had a question
> about the # of active nmethods that get thus linked into your marked
> list. Is this a fairly small number? Or fairly large?
> 
> If large, then perhaps an alternative implementation might be worth
> considering where each worker thread maintains its own local list
> (head) to which it links the just claimed nmethod rather than to the
> single global list. This would reduce contention for a global head
> and would avoid one CAS per nmethod  claimed.
> Then, in the epilogue (which would have to be appropriately sited
> in the per-worker marking code), the workers could run down their
> local lists in parallel, thus avoiding a serial bottlebeck.
> As I said, it depends on the expected volume of these active
> nmethods. Perhaps they are sufficiently small (even as # active
> threads in the application scales up) for this not to become
> a scalability issue in the future?
> 
>>   http://cr.openjdk.java.net/~jrose/6863023/webrev.03 (full webrev)
>>
>> These changes fix some double-tracing bugs, but they are not correct.  
>> However, they are not right yet.  What I'm asking for now is (a) 
>> further advice for structuring the nmethod-walking code correctly, and 
>> (b) help spotting bugs.
> 
> Any hints on the kinds of bugs you may be seeing? My first overview of
> the structure of yr changes looks fine, but i will do a second, more
> careful, pass to see if i can spot any potential issues.
> 
> later.
> -- ramki
> 
>>
>> -- John
> 
> 



More information about the hotspot-gc-dev mailing list