RFR: Some more descriptive names, removed redundant asserts. Request for comments. [v2]

[Coleen Phillimore]

On Tue, 5 Apr 2022 19:34:44 GMT, Ron Pressler <rpressler at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Revert untemplatization
>
> src/hotspot/cpu/aarch64/continuation_aarch64.inline.hpp line 90:
> 
>> 88:     // we need to re-read fp because it may be an oop and we might have had a safepoint in finalize_freeze,
>> 89:     // after constructing f.
>> 90:     // This comment doesn't make sense since we don't reread fp
> 
> See x86 response

ok

> src/hotspot/cpu/x86/continuation_x86.inline.hpp line 104:
> 
>> 102:     // we need to re-read fp because it may be an oop and we might have had a safepoint in
>> 103:     // finalize_freeze, after constructing f.
>> 104:     // This comment doesn't make sense since we don't reread fp
> 
> We read the spilled fp from the frame into `fp`.

Line 113/117 below is why it didn't make sense to me.  We have to read fp to create the frame below. Oh but instead of passing f.fp() we have to read the oop out of the frame.  I'll add "out of the frame" to the comment.

> src/hotspot/share/runtime/continuation.cpp line 1001:
> 
>> 999:   );
>> 1000:   freeze_result try_freeze_fast(intptr_t* sp, bool chunk_available);
>> 1001:   bool freeze_fast(intptr_t* top_sp, bool chunk_available);
> 
> We must not branch on the fast path (i.e. we might have budget to branch once or maybe twice). For now, this needs to remain a template parameter. It has no significant impact on build time and is just as easy to read. This kind of thing is how we managed to avoid writing it in assembly.

This template seemed dubiously helpful and now we have 4 of this huge function, which the compiler may be able to throw away parts of.  Aside, can't see how the ConfigT parameter is used here.  Maybe we don't need that?   I'll revert it but we really dislike these templates and think that the C++ compiler can do just as good as a job optimizing this code.  Templates are not easy to read and even worse to debug, and essentially add a level of indirection for understanding the code.  The syntax and compilation messages are awful!  We need a set of benchmarks to justify this coding style.

> src/hotspot/share/runtime/continuation.cpp line 2222:
> 
>> 2220:   }
>> 2221: 
>> 2222:   // Comment needed: why is 500? Related to 300 in can_thaw_fast?
> 
> It's a heuristic. Below this number, we thaw the whole chunk. Above it, we thaw one frame. 500 seemed to work ok, but might need to be adjusted after deeper profiling.

Unrelated to 300 above then?  Seems like we've stack overflow checked for 300 words so that would be a good slop number for both?

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

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