Request for review JDK-8140588 - Internal Error: gc/g1/ptrQueue.hpp:126 assert(_index == _sz) failed: invariant: queues are empty when activated

[Per Liden]

Hi,

On 2017-01-05 00:36, Kim Barrett wrote:
>> On Dec 24, 2016, at 10:04 AM, Alexander Harlap <alexander.harlap at oracle.com> wrote:
>>
>> Please review change for JDK-8140588 - Internal Error: gc/g1/ptrQueue.hpp:126 assert(_index == _sz) failed: invariant: queues are empty when activated
>>
>> Change is located at http://cr.openjdk.java.net/~aharlap/8140588/webrev.00/
>>
>> It implements Per Liden fix for rechecking value of marking in progress value inside C1 g1_pre_barrier code.
>>
>> Also here is optimization described by Thomas Schatzl - do recheck only if patching involved.
>
> I wonder whether it is worth the effort of having distinct stubs for
> the two cases, or just unconditionally perform the recheck in the
> existing stub.  Thomas said he found no performance issue with the
> simpler version.

Agree. If there's no measurable performance gain by having two stubs 
(given the other things we do in this path I'd be surprised if there 
were) I'd vote for keeping it simple here and have one stub. That would 
also "fix" Kim's last comment below.

cheers,
Per

>
> ------------------------------------------------------------------------------
> Throughout, copyright dates need updating.
>
> ------------------------------------------------------------------------------
> cpu/aarch64/vm/c1_CodeStubs_aarch64.cpp
>  373   Runtime1::StubID id = patch_code() == lir_patch_none ? Runtime1::g1_pre_barrier_slow_id : Runtime1::g1_pre_barrier_slow_with_recheck_id;
>
> Assuming we stay with the pair of stubs, this ought to be packaged up
> as a helper function.
>
> Other occurrences:
> src/cpu/arm/vm/c1_CodeStubs_arm.cpp
> src/cpu/ppc/vm/c1_CodeStubs_ppc.cpp
> src/cpu/s390/vm/c1_CodeStubs_s390.cpp
> src/cpu/sparc/vm/c1_CodeStubs_sparc.cpp
> src/cpu/x86/vm/c1_CodeStubs_x86.cpp
>
> ------------------------------------------------------------------------------
>