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