RFR: 8301404: Factor out os::malloc with os::realloc common code, so that we only have 1 code path [v8]

[Thomas Stuefe]

On Tue, 1 Apr 2025 12:51:03 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> This is the 2nd time I am proposing this (controversial?) change, but this time I do have performance numbers, which indicate no change in speed (using NMTBenchmark from #23786):
>> 
>> proposed:
>> 
>> time:72,642,827[ns]
>> [samples:807,804] [NMT headers:382,064]
>> [malloc#:588,703] [realloc#:12,462] [free#:206,639]
>> memory requested:57,274,288 bytes, allocated:69,004,800 bytes
>> malloc overhead:4,853,360 bytes [8.47%], NMT headers overhead:6,877,152 bytes [12.01%]
>> existing code:
>> 
>> time:73,085,446[ns]
>> [samples:807,804] [NMT headers:382,064]
>> [malloc#:588,703] [realloc#:12,462] [free#:206,639]
>> memory requested:57,274,288 bytes, allocated:69,004,800 bytes
>> malloc overhead:4,853,360 bytes [8.47%], NMT headers overhead:6,877,152 bytes [12.01%]
>> Note: the NMTBenchmark reports realloc(nullptr) as mallocs(), which is why both versions show the same count for mallocs/reallocs.
>> 
>> The performance is virtually the same where I sampled each test 30 times and took the best (the shortest).
>> 
>> This proposed change factors out the common code and simplifies both os::malloc and os::realloc. We were able to reduce malloc from 44 lines down to 8 (saving of 36 lines) and realloc from 84 to 55 (29 lines).
>> 
>> To me the most important part here is that we reduce the number of times that NMT has to interact with the native allocation code.
>
> Gerard Ziemski has updated the pull request incrementally with one additional commit since the last revision:
> 
>   revert last checkin - wrong issue

I have to think this through. I know that this code can be deceptively tricky, and looks deceptively simple, but a lot of thought has been put into the details of the existing implementation. Plus, this code is not well covered with regression tests.

For those reasons alone, I'd rather avoid changes like these. They save 36 lines of code, but the simplicity here is really subjective, and the review work needed to be put into this is time we probably won't ever get back from these simplifications. 

Just my 5 cent. I am not against refactoring code, far from it, but there are oh so many places in the JVM that would benefit more from some cleanup love (e.g. hotspot arenas).

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

PR Comment: https://git.openjdk.org/jdk/pull/24189#issuecomment-2769796707