RFR: Small fixes - shipment one

Robbin Ehn rehn at openjdk.java.net
Tue Mar 22 15:38:02 UTC 2022


On Tue, 22 Mar 2022 14:33:12 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Reducing the number of variables, the number of assignment to each variable, white-spaces, renamed some variables, removed unnecessary template, etc...
>> Except the removal of template 'bottom', the code is the same. (assert is moved up one level)
>> 
>> Passes 1-3 (no new failures)
>> It seem to be slightly faster than before, but at least on par.
>> 
>> This is not complete at all, but to avoid merge conflict I want to ship, and also make someone happy about removing a template.
>
> src/hotspot/share/runtime/continuation.cpp line 180:
> 
>> 178: #define VERIFY_CONTINUATION(cont) verify_continuation((cont))
>> 179: NOINLINE static bool verify_stack_chunk(oop chunk) { return InstanceStackChunkKlass::verify(chunk); }
>> 180: #define VERIFY_STACK_CHUNK(chunk) verify_stack_chunk((chunk))
> 
> Thanks for fixing this.  I know Ron likes to see the asserts have a line number of the caller but there's a PR to fix this and almost all of use some debugger to see where the assert comes from in the call stack, if it's not already in the hs_err file.

A better approach would be to print the line number in the assert:

InstanceStackChunkKlass::verify(chunk, __LINE__);
The assert would be assert(good, "%u: ....", lineno, ....) ;

> src/hotspot/share/runtime/continuation.cpp line 1160:
> 
>> 1158:     // properties of the continuation on the stack; all sizes are in words
>> 1159:     intptr_t* const stack_top     = top_sp + ContinuationHelper::frame_metadata;
>> 1160:     const int       stack_argsize = _cont.argsize();
> 
> I like this spelled out in the places it's used.  I wonder if _cont.argsize() is the stacksize of the stack chunk oops.  There are no comments really what it is.
> cont_size seems to be the stack size of the entire continuation, ie the sum of all the stack chunk oops?  Dunno.  We're going to need some comments.

_cont argsize is normally: inline int StackChunkFrameStream<frame_kind>::stack_argsize()
But it depends what we mean, a single intepreted frame must have the args from caller to be mounted.
But if caller is already mounted we already have those arguments.

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

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


More information about the loom-dev mailing list