RFR: Improve C2 barrier filtering: heap-stable check should test for HAS_FORWARDED only

[Roman Kennke]

Great! Looks good!

Roman

Am 20.07.2018 um 13:04 schrieb Aleksey Shipilev:
> http://cr.openjdk.java.net/~shade/shenandoah/c2-has-forwarded/webrev.01/
> 
> This irritated me for quite a while. Our current strategy is to check that gc-state is zero, and
> claim the heap is stable then. However, this has a significant drawback in barriers filtering, when
> marking is active on heap that has no forwarded objects, and gc-state = 2 (marking): WB enters the
> mid-path and does RB, even though we know there are no forwarded objects. SATB barriers jumps to
> midpath even when marking is not enabled, but we have e.g. forwarded objects or evac/update-refs in
> progress.
> 
> The improvement is to redefine what is "heap stable" in C2: look for HAS_FORWARDED bit only. Since
> this check shape had been changed, there is no reason to have the gc-state = 0 check before SATB
> barriers too. If we consider further barrier coalescing, it would need to common tests for gc-state
> = 1 (has_forwarded) and gc-state = 2 (marking) under new gc-state = 0 (idle) branch. But until that
> happens, WB fast-path performance is important.
> 
> Look at disassemblies here:
>   http://cr.openjdk.java.net/~shade/shenandoah/c2-has-forwarded/baseline.perfasm
>   http://cr.openjdk.java.net/~shade/shenandoah/c2-has-forwarded/patched.perfasm
> 
> WB fastpath used to be:
> 
>        0x00007f966ca34654: cmpb   $0x0,0x20(%r15)       ; <--- jump if anything is happening
>   ╭    0x00007f966ca34659: jne    0x00007f966ca3468a
>   │↗   0x00007f966ca3465b: mov    %rdx,%rbx
>   ││    ...
>   ↘│   0x00007f966ca3468a: testb  $0x14,0x20(%r15)
>    │   0x00007f966ca3468f: jne    0x00007f966ca346de
>    │   0x00007f966ca34691: mov    -0x8(%rdx),%r8        ; <--- meet the RB (mostly useless)
>    │   0x00007f966ca34695: mov    %r8,%rdx
>    ╰   0x00007f966ca34698: jmp    0x00007f966ca3465b
> 
> WB fastpath now:
> 
>        0x00007fd204a341d4: testb  $0x1,0x20(%r15)       ; <--- only jump if HAS_FORWARDED
>   ╭    0x00007fd204a341d9: jne    0x00007fd204a3420a
>   │↗   0x00007fd204a341db: mov    %rdx,%rbx
>   ││   ...
>   ↘│   0x00007fd204a3420a: testb  $0x14,0x20(%r15)
>    │   0x00007fd204a3420f: jne    0x00007fd204a34256
>    │   0x00007fd204a34211: mov    -0x8(%rdx),%r8
>    │   0x00007fd204a34215: mov    %r8,%rdx
>    ╰   0x00007fd204a34218: jmp    0x00007fd204a341db
> 
> 
> SATB fastpath used to be:
> 
>        0x00007f966ca3466c: cmpb   $0x0,0x20(%r15)      ; <--- jump if anything is happening
>   ╭    0x00007f966ca34671: jne    0x00007f966ca3469e
>   │↗↗  0x00007f966ca34673: mov    %rbp,%r11
>   │││    ..
>   ↘││  0x00007f966ca3469e: testb  $0x2,0x20(%r15)      ; <--- jump back if that was not marking
>    ╰│  0x00007f966ca346a3: je     0x00007f966ca34673
>     │  0x00007f966ca346a5: mov    0xc(%rdx),%r10d      ; satb barrier head
>     │  0x00007f966ca346a9: test   %r10d,%r10d
>     ╰  0x00007f966ca346ac: je     0x00007f966ca34673
> 
> 
> SATB fastpath now:
> 
>        0x00007fd204a341ec: testb  $0x2,0x20(%r15)      ; <--- straight test for MARKING
>    ╭   0x00007fd204a341f1: jne    0x00007fd204a3421e
>    │↗  0x00007fd204a341f3: mov    %rbp,%r10
>    ││..
>    ↘│  0x00007fd204a3421e: mov    0xc(%rdx),%r11d      ; satb barrier head
>     │  0x00007fd204a34222: test   %r11d,%r11d
>     ╰  0x00007fd204a34225: je     0x00007fd204a341f3
>        0x00007fd204a34227: mov    0x40(%r15),%r10
>        0x00007fd204a3422b: mov    %r11,%rdi
> 
> 
> Testing: tier3_gc_shenandoah, benchmarks
> 
> Thanks,
> -Aleksey
>