RFR: 8297718: Make NMT free:ing protocol more granular [v2]
Thomas Stuefe
stuefe at openjdk.org
Wed Dec 7 17:07:29 UTC 2022
On Wed, 7 Dec 2022 16:41:36 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:
> > Matter of taste. I dislike this kind of coding, even though it is common in hotspot, where you have a compound function and a bunch of flags (often uncommented) fine-controlling its behavior. You then end up having to read the code to understand its behavior exactly.
>
> Currently we have gone **from:**
>
> ```
> const size_t old_size = MallocTracker::malloc_header(memblock)->size();
> void* const old_outer_ptr = MemTracker::record_free(memblock);
> ```
>
> **to:**
>
> ```
> MallocHeader* header = MallocTracker::malloc_header(memblock);
> header->assert_block_integrity(); // Assert block hasn't been tampered with.
> const MallocHeader::FreeInfo free_info = header->free_info();
> header->mark_block_as_dead();
> ```
>
> But the change could have been just **from:**
>
> ```
> const size_t old_size = MallocTracker::malloc_header(memblock)->size();
> void* const old_outer_ptr = MemTracker::record_free(memblock);
> ```
>
> **to:**
>
> ```
> MallocHeader* header = MemTracker::record_free(memblock, false);
> const MallocHeader::FreeInfo free_info = header->free_info();
> ```
>
> Where `header` related APIs are nicely encapsulated and hidden away.
>
As I wrote, it's a matter of taste. Sometimes a bit verbose is a good tradeoff if it makes for easy understanding.
So, operation x does A B C D, but if I pass false, its A B D. Okay, what if you need a different set of operations? Maybe you need just A and D? Give it another parameter? One for every permutation? That is exactly what often happens, and then you end up with long chain of arguments, each one subtly changing behavior, many of them often one-trick-ponies that are only used at one site.
I quickly find this more complex than another line of plain code at the call site would be. You soon arrive at a point where you cannot tell from the call site what will happen. You have to dive into the code of the subroutine. Especially if both call and parameters are badly named and undocumented, which is often the case within hotspot.
-------------
PR: https://git.openjdk.org/jdk/pull/11390
More information about the hotspot-runtime-dev
mailing list