RFR: Freeze functions [v3]

[Coleen Phillimore]

On Wed, 20 Apr 2022 10:49:25 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> Freeze functions
>
> Robbin Ehn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Rename

Hard to tell with diffs but I think this looks good.  I like the refactoring of freeze_fast_copy.

src/hotspot/share/runtime/continuationFreezeThaw.cpp line 338:

> 336:   bool _barriers;
> 337:   const bool _preempt; // used only on the slow path
> 338:   const intptr_t * const _frame_sp;

Is this the sp of the first frame on the stack that we are going to freeze?

src/hotspot/share/runtime/continuationFreezeThaw.cpp line 347:

> 345:   intptr_t* _cont_stack_top;
> 346:   intptr_t* _cont_stack_bottom;
> 347:   int _cont_size;

I know these are in logical grouping and it's a StackObj, but can you move the two bools to after cont_size to minimize alignment gaps.  Also there are three sizes now in this class.  Can we have bigger comments for what each means in the declaration?  I think this will be helpful for the abstraction.

src/hotspot/share/runtime/continuationFreezeThaw.cpp line 509:

> 507:   } else {
> 508:     stackChunkOop chunk = allocate_chunk(_cont_size + frame::metadata_words);
> 509:     if (freeze_fast_new_chunk(chunk)) {

It seems odd that freeze_fast_existing_chunk() doesn't pass a chunk but freeze_fast_new_chunk does.  Should the allocate_chunk be inside of freeze_fast_new_chunk?

src/hotspot/share/runtime/continuationFreezeThaw.cpp line 539:

> 537:   assert(StubRoutines::cont_doYield_stub()->frame_size() == frame::metadata_words, "");
> 538: 
> 539:   int ret_size = _cont_size;

ret_size is the total size that you need to freeze right?  Maybe call it total_size_needed or something better.

src/hotspot/share/runtime/continuationFreezeThaw.cpp line 608:

> 606:   }
> 607: 
> 608:   chunk->set_max_size(_cont_size);

This variable _cont_size is really strange.  Above we add in argsize for total size that we need to freeze, but here we set max_size to cont_size.  We should go through and see what we add and subtract from cont_size in various places and see what size is what (and if it's easier to figure out what's correct).  After this patch.
I assume that this patch is mostly code motion and doesn't change the values in places, which is good.

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

Marked as reviewed by coleenp (Committer).

PR: https://git.openjdk.java.net/loom/pull/156