[9] RFR(S): 8034839: jvm hangs with gc/gctests/LoadUnloadGC test

[Christian Thalinger]

On Feb 21, 2014, at 11:27 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> Lets discuss it on hotspot-dev.
> 
> Note the current hashtable allocates only in c_heap. Albert added hashtable which can allocate in thread local resource area for temporary table and c_heap for long live table.
> 
> Albert,
> 
> So you restored code in dependencies.?pp to one before 7194669 fix. Right?
> 
> I think you need to follow GrowableArray example to name parameter "bool C_heap = false" instead of "bool resource_mark". It should be saved in a field because you need to free c_heap in destructor if C-heap is used:
> 
> ~GrowableArray()  { if (on_C_heap()) clear_and_deallocate(); }
> 
> Also I think you should avoid call to contains(item) in add() to avoid doing the same thing twice.

…and you should stick to either item or element:

+ template<class T, class F> bool GenericHashtable<T, F>::add(T* item) {
+ template<class T, class F> bool GenericHashtable<T, F>::contains(T* element) {

> 
> You should implement remove().
> 
> Thanks,
> Vladimir
> 
> On 2/21/14 12:04 AM, Albert wrote:
>> Hi,
>> 
>> could I get reviews for this small patch?
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8034839
>> 
>> Problem: The problem is that the patch (7194669) - which was supposed to
>> speed-up dependency checking
>>                causes a performance regression. The reason for the
>> performance regression is that most dependencies
>>                are unique, so we have the overhead of determining if
>> the dependency is already checked plus the
>>                overhead of dependency checking. The overhead of
>> searching is significant, since we perform
>>                a linear search on 6000+ items each time.
>> 
>> Solution: Use a hashtable instead of linear search to lookup already
>> checked dependencies. The new hashtable
>>                is very rudimentary. It provides only the required
>> functionality to solve this bug. However, the functionality
>>                can be easily extended as needed.
>> 
>> Testing: jprt, failing test case, nashorn. The failing test case
>> completes in approx. the same time as before 7194669.
>>              For nashorn + Octane, this patch yields the following
>> times spent for dependency checking:
>> 
>>               with this patch:  844s
>>                        7194669: 1080s
>>            before 7194669: 5223s
>> 
>> webrev: http://cr.openjdk.java.net/~anoll/8034939/webrev.00/
>> 
>> Thanks,
>> Albert
>>