RFR: 8158045: Improve large object handling during evacuation

Kim Barrett kim.barrett at oracle.com
Fri Aug 28 00:28:30 UTC 2020


> On Aug 27, 2020, at 12:24 PM, Albert Yang <albert.m.yang at oracle.com> wrote:
> 
> Hi,

Thanks for looking at this.

> ```
>  81   bool _old_gen_is_full;
>  82
>  83   int _objarray_scan_chunk_size;
>  84   int _objarray_length_offset_in_bytes;
> ```
> 
> I think these three fields can be reordered/shrunk to be more compact, something like:
> 
> ```
> int32_t _objarray_scan_chunk_size;
> uint8_t _objarray_length_offset_in_bytes;
> bool _old_gen_is_full;
> ```

I don't think space considerations are particularly important here.  I think
it's more important that the types of these cached values match the types
from where they are obtained.

> Not directly changed by this patch, but close enough. In `G1ParScanThreadState::do_copy_to_survivor_space`:
> 
> ```
> if (forward_ptr == NULL) {
>  // 60 lines; partially covered in this patch
> } else {
>  _plab_allocator->undo_allocation(dest_attr, obj_ptr, word_sz, node_index);
>  return forward_ptr;
> }
> ```
> 
> could be turned into the following using early-return, which feels nicer, IMO.
> 
> ```
> if (forward_ptr) {
>  _plab_allocator->undo_allocation(dest_attr, obj_ptr, word_sz, node_index);
>  return forward_ptr;
> }
> // 60 lines
> ```

Using `if (forward_ptr) {` is an implicit boolean, so shouldn't be used (per
the Style Guide).

I think the current code layout is based on importance / likelihood,
focusing the reader on what matters and putting the unlikely but not trivial
case out of the way.  That said, I kind of agree the undo case is maybe a
little too out of the way.  I'd rather not rearrange this right now, as I
have other changes in this area in development, but I'll keep it in mind for
a possible followup.




More information about the hotspot-gc-dev mailing list