[Fwd: Request for reviews (M): 6973963: SEGV in ciBlock::start_bci() with EA]

Vladimir Kozlov vladimir.kozlov at oracle.com
Tue Aug 3 05:45:10 UTC 2010


I updated webrev:

http://cr.openjdk.java.net/~kvn/6973963/webrev.01

Additional changes:

1. Used new option StressRecompilation instead of CompileTheWorld.
    I am still not sure if it is OK since we never test our stress options.

2. Added new allocation type: STACK_OR_EMBEDDED for cases when new() is not called.

3. Added ResourceObj destructor to zap _allocation field.

4. Added assert into get_allocation_type() to check that 'this' address is still encoded in _allocation.
    Found several cases where it was not true, have to add copy constructor and assignment operator.

5. Moved all new methods with asserts into allocation.cpp.

6. The added assert failed for CodeBuffer since it destroys itself inside destructor before ResourceObj destructor called.
    Moved Copy::fill_to_bytes(badResourceValue) into CodeBuffer::operator delete().

7. Replaced PhaseCFG::_node_latency field with pointer since it is valid only inside resource mark in GlobalCodeMotion().

Thanks,
Vladimir

Vladimir Kozlov wrote:
> Thank you, Tom
> 
> RESOURCE_AREA case combines 3 cases:
> 
> GrowableArray<int> *foo = new GrowableArray<int>(size); // resource 
> array allocation
> 
> GrowableArray<int> foo; // stack allocation
> 
> class A : public ResourceObj {
>   GrowableArray<int> foo; // embedding allocation
> }
> 
> In all this cases GrowableArray::_data array is allocated in thread local
> resource area. GrowableArray calls all these cases as stack allocation
> and I followed this naming.
> 
> But you are right I can separate 2 last cases into "stack" (stack or 
> embedding)
> allocation, I still have 1 bit in allocation_type :)
> 
> Thanks,
> Vladimir
> 
> Tom Rodriguez wrote:
>> So doesn't this triple the amount of time taken by CTW?  That seems 
>> kind of extreme for the default mode.  Maybe it should be under a 
>> flag?  The allocation.hpp changes look a little sketchy. 
>> +     if (~(_allocation | allocation_mask) != (uintptr_t)this) {
>> +       set_allocation_type((address)this, RESOURCE_AREA);
>>
>> Why is this case called RESOURCE_AREA?  Isn't this the stack or 
>> embedding case?
>>
>> This doesn't make sense either.
>>
>> !   bool allocated_on_stack()     { return get_allocation_type() == 
>> RESOURCE_AREA; }
>>
>> Anyway, the existing logic around this seemed sketchy so I can't quite 
>> say whether this is better or not.  I'll have to leave that to someone 
>> else.
>>
>> The GrowableArray changes themselves look fine.
>>
>> tom
>>
>> On Aug 2, 2010, at 3:01 PM, Vladimir Kozlov wrote:
>>
>>> Forwarding to GC and Runtime groups since it is common code.
>>>
>>> Vladimir
>>>
>>> -------- Original Message --------
>>> Subject: Request for reviews (M): 6973963: SEGV in 
>>> ciBlock::start_bci() with EA
>>> Date: Mon, 02 Aug 2010 14:57:25 -0700
>>> From: Vladimir Kozlov <vladimir.kozlov at oracle.com>
>>> To: hotspot compiler <hotspot-compiler-dev at openjdk.java.net>
>>>
>>> http://cr.openjdk.java.net/~kvn/6973963/webrev
>>>
>>> Fixed 6973963: SEGV in ciBlock::start_bci() with EA
>>>
>>> I added stress recompilation during CompileTheWorld and found this case.
>>> It is similar to 6968368. BCEscapeAnalyzer::do_analysis() calls
>>> ciMethod::get_method_blocks() which calls constructor ciMethodBlocks.
>>> This constructor allocates GrowableArray elements on stack (thread
>>> local resource area). As result when the method recompiled without EA
>>> _blocks->_data is NULL.
>>>
>>> Solution:
>>> Added stress recompilation during CompileTheWorld: recompile with
>>> subsume_loads = false and do_escape_analysis = false.
>>> Added more checks into ResourceObj and growableArray to verify 
>>> correctness
>>> of allocation. I have to relax the new assert in GrowableArray when
>>> elements are allocated on arena to allow allocattion of GrowableArray 
>>> object
>>> as a part of an other object (for example, in ConnectionGraph and 
>>> SuperWord).
>>>
>>> Tested with failed cases, CTW.
>>



More information about the hotspot-gc-dev mailing list