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