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

[Stefan Karlsson]

On Mon, 31 Mar 2025 20:44:25 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:
> 
>   pass explicit mem_tag value, skip exec parameter when false

I took a deeper look at the proposed patch and I'm still not convinced that this is a net positive cleanup. The added shared code now contains new branches to sort out the difference between malloc and realloc.

FWIW, I poked around at the original code to see what it would look to go the other way. Push NMT-specific code deeper down so that the higher-level functions get clearer and maybe easier to maintain. If you want to take a look, I've a branch for that here:
https://github.com/openjdk/jdk/compare/master...stefank:jdk:24189_alternative

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

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