review request (S) 6866585 debug code in ciObjectFactory too slow

[Tom Rodriguez]

I'm not against putting it under a flag but that just seems equivalent  
to deleting it.  It's yet another flag no one knows about or uses.   
Could we just use CollectedHeap::total_collections as a counter guard  
for triggering the code, i.e. do a full verification or the sort order  
if the value changes?  If we do that, I'd be fine with the spot  
verification you added on insert.  If that won't work I guess putting  
it under a flag is acceptable.

tom

On Aug 6, 2009, at 2:15 PM, John Coomes wrote:

> Y.S.Ramakrishna at Sun.COM (Y.S.Ramakrishna at Sun.COM) wrote:
>> (Please take my comments with the requisite dose of salt because
>> i know little about these CIObjects, so i may be talking nonsense
>> in this context :-)
>>
>> How big could the (unified) table get? (Clearly in the case of the  
>> test that
>> brought forth the problem it must become quite large, but perhaps  
>> that's
>> an extreme case.) Still, I'd be wary of anything that
>> requires a large table re-sort at each full gc, let alone at each  
>> scavenge...
>> (BTW, what do you do with non-perm stuff today where order is not  
>> preserved
>> across GCs? why not do that for all objects rather than rely on  
>> ordering
>> for the perm subset?) Of course, if the table is small, may be my  
>> concern is moot.
>
> FWIW, the current table gets to a little over 64K elements with my
> test case.  get() is called 3 times at each value of length (from 1 -
> 64K+), and each call does two iterations over the table.  It's a lot.
>
> Long term, I think the GC should notify the CI when perm gen objects
> may have been reordered.  The CI should just record the notification,
> and later re-sort or do whatever is necessary on the next
> get()/insert()/whatever.
>
> As Ramki mentioned, none of our current GCs would have to do this, we
> just have to worry about future GCs.  I think a reasonable solution
> would be to have current GCs do the (unnecessary) notification
> whenever perm gen is collected in debug builds.  That would exercise
> the code without excessive overhead, and also document the dependency
> in the code.  I would expect a new collector to start by cloning an
> existing collector.
>
> In the short-term, I just want to be able to check in my test case.
> How about if I restore the debugging code, but put it under control of
> the CIObjectFactoryVerify option?
>
> I'll also file a bug to capture the comments.
>
> -John
>
>> On 08/06/09 11:36, Tom Rodriguez wrote:
>>>>>> Would it be possible to put the check into the GC epilogue?
>>>>>> Probably not, since ciEnv guys are not statically allocated and  
>>>>>> not
>>>>>> easy to enumerate (one per active compile-thread task).  I  
>>>>>> guess I
>>>>>> do like the idea of checking a GC tick counter from the CI, to
>>>>>> throttle the paranoid check.  Best of all would be to have some
>>>>>> explicit linkage from the GC epilogue code to the CI, which  
>>>>>> somehow
>>>>>> documents and enforces the order invariance.  You can see why  
>>>>>> this
>>>>>> is awkward:  It is an invariant shared by the CI and the GC.
>>>>
>>>> Yes, i agree, if it's one per active compile task, may be it's  
>>>> best to
>>>> use
>>>> yr strategy of eliding the check unless the full gc count changes.
>>>
>>> John, couldn't this strategy allow us to reunify the handling of  
>>> oops
>>> into a single table instead of needing the NonPermOop stuff?  If we
>>> simply re-sort the table when needed that code becomes simple  
>>> again.  It
>>> also makes us impervious to reordering concerns.
>>>
>>> tom
>>>
>>>>
>>>> -- ramki
>>>>
>>>>> -- John
>>>>
>>>
>>
>