RFR: 8371667: Shenandoah: Re-design alloc request type enum for better efficiency and cleaner code

[Aleksey Shipilev]

On Wed, 12 Nov 2025 01:29:53 GMT, Xiaolong Peng <xpeng at openjdk.org> wrote:

> Current alloc request type enum:
> 
>   enum Type {
>     _alloc_shared, // Allocate common, outside of TLAB
>     _alloc_shared_gc, // Allocate common, outside of GCLAB/PLAB
>     _alloc_cds, // Allocate for CDS
>     _alloc_tlab, // Allocate TLAB
>     _alloc_gclab, // Allocate GCLAB
>     _alloc_plab, // Allocate PLAB
>     _ALLOC_LIMIT
>   };
> 
> With current design, we have to use switch statement in multiple places resulting in unnecessary branches, for instance the function is_mutator_alloc:
> 
> 
>   inline bool is_mutator_alloc() const {
>     switch (_alloc_type) {
>       case _alloc_tlab:
>       case _alloc_shared:
>       case _alloc_cds:
>         return true;
>       case _alloc_gclab:
>       case _alloc_plab:
>       case _alloc_shared_gc:
>         return false;
>       default:
>         ShouldNotReachHere();
>         return false;
>     }
>   }
> 
> 
> 
> In PR, I have re-designed the enum to make the function like is_mutator_alloc much simpler by making the values of the enum follow two simple rules:
> 1. Smaller value for mutator alloc, larger value for gc alloc; GC alloc types are always greater than any of mutator alloc types.
> 2. Odd for lab, even number for non-lab
> 
> Three functions have been simplified to one-line impl w/o branches in machine code:
> 
> 
>   inline bool is_mutator_alloc() const {
>     return _alloc_type <= _alloc_shared;
>   }
> 
>   inline bool is_gc_alloc() const {
>     return _alloc_type >= _alloc_shared_gc;
>   }
> 
>   inline bool is_lab_alloc() const {
>     return (_alloc_type & 1) == 1;
>   }
> 
> 
> I didn't check compiled assemble code  of hotspot, in instead, I wrote similar/equivalent code and compile with gcc for comparison using godbolt.org:  
> 
> bool is_lab_alloc(int alloc_type) {
>     return (alloc_type & 1) == 1;
> }
> 
> bool is_lab_alloc_switch(int alloc_type) {
>     switch (alloc_type) {
>         case 0:
>         case 2: 
>         case 4:
>           return false;
>         case 1:
>         case 3:
>         case 5:
>           return true;
>         default:
>           throw "Should not reach here";
> 
>     }
> }
> 
> x86_64 assembly code (https://godbolt.org/z/h7xfz8PaT):
> 
> is_lab_alloc(int):
>         push    rbp
>         mov     rbp, rsp
>         mov     DWORD PTR [rbp-4], edi
>         mov     eax, DWORD PTR [rbp-4]
>         and     eax, 1
>         and     eax, 1
>         pop     rbp
>         ret
> .LC0:
>         .string "Should not reach here"
> is_lab_alloc_switch(int):
>         push    rbp
>         mov     rbp, rsp
>         sub     rsp, 16
>         mov     DWORD PTR [rbp-4], edi
>         cmp     DWORD PTR [rbp-4], 5
>         je      .L...

Honestly, I don't believe switching from explicit switch cases to bit manipulation is that much readable here. If you want to pursue this, then maybe do the proper bitmask manipulation, something like:


 // bit 0: mutator (0) or GC (1) alloc 
 // bit 1: LAB (0) or shared (1) alloc 
 // bit 2: if LAB, then GCLAB (0) or PLAB (1)
 // bit 3: if mutator, then normal (0) or CDS (1)
 typedef int AllocType;

 constexpr int bit_gc_alloc = 1 << 1;
 constexpr int bit_lab_alloc = 1 << 2;
 constexpr int bit_plab_alloc = 1 << 3;
 constexpr int bit_cds_alloc = 1 << 4;
 ...
 const bool is_lab_alloc(AllocType type) {
    return (type & bit_lab_alloc) != 0;
 }
 
 constexpr AllocType _alloc_tlab = bit_lab_alloc;
 constexpr AllocType _alloc_plab = bit_gc_alloc | bit_lab_alloc | bit_plab_alloc;
 ...


Remains to be seen what is the most sensible encoding scheme here.

-------------

PR Review: https://git.openjdk.org/jdk/pull/28247#pullrequestreview-3452564118