RFR: 8355481: Clean up MHN_copyOutBootstrapArguments [v3]

Coleen Phillimore coleenp at openjdk.org
Fri May 9 11:47:54 UTC 2025


On Fri, 9 May 2025 11:32:47 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Hi,
>> 
>> I'd like to integrate this simplification of the code for this loop.
>> 
>> We used to have:
>> 
>> ```c++
>> if (start < 0) {
>>   for (int pseudo_index = -4; pseudo_index < 0; pseudo_index++) {
>>     if (start == pseudo_index) {
>>       if (start >= end || 0 > pos || pos >= buf->length())  break;
>>       // ...
>>     }
>>     start++;
>>   }
>> }
>> 
>> 
>> That's exactly the same as:
>> 
>> 
>> int min_end = MIN2(0, end);
>> while (-4 <= start && start < min_end) {
>>   if (pos >= buf->length()) break;
>>   // ...
>>   start++;
>> }
>> 
>> 
>> but the latter looks like a conventional loop.
>> 
>> I'd consider this a basic cleanup, which is worth doing in the name of maintainability.
>> 
>> I would have liked to change the `-4` to `-1` into actual names, but I've no clue where those come from. It doesn't seem worth it to change them if they just happen to be a kludge relying on internal details, or something like that.
>> 
>> Testing: GHA Tier1
>
> Johan Sjölen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Delete unused code

src/hotspot/share/prims/methodHandles.cpp line 1260:

> 1258:       THROW_MSG(vmSymbols::java_lang_InternalError(), "bad index info (1)");
> 1259:   }
> 1260:   objArrayHandle buf(THREAD, (objArrayOop) JNIHandles::resolve(buf_jh));

Do you want to assert about the value of 'start'  and 'end'?  This became a nice cleanup.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24825#discussion_r2081500928


More information about the hotspot-dev mailing list