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

[Vladimir Kozlov]

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.
>