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