RFR: 8301149: Parallel: Refactor MutableNUMASpace::update_layout
Leo Korinth
lkorinth at openjdk.org
Tue Jan 31 18:48:49 UTC 2023
On Thu, 26 Jan 2023 12:15:56 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:
> Small cleanup to simplify the structure.
>
> Test: hotspot_gc on boxes with two NUMA nodes.
Hi, I have taken a fast look at this, and I realise I am not qualified to review this. However I have some observations I want to share:
1) The code looks *much* better after your change, *and* quadratic complexity is removed!!!
2) The old code only allocated new LGRPSpace if not found (on updates), could this be a problem (I guess not from an allocation cost perspective, but maybe from eventual state now being reset)?
3) From my understanding, the code is called from two locations. The first location is the constructor `MutableNUMASpace::MutableNUMASpace(size_t alignment)` only once (per process) called from `ParallelScavengeHeap::initialize`. The second location is `MutableNUMASpace::update()`. However, in this case `force` is `false` and `changed` is false in every operating system implementation I can find. So I wonder if the code can actually be tested in the case of an `update()` where my second observation applies.
4) If observation 4 is correct, the code is dead from an update perspective, and if that is true, that means that the code can only be tested from an initialisation point of view. It also means that we have (quite a bit) logic for code that is dead.
-------------
PR: https://git.openjdk.org/jdk/pull/12216
More information about the hotspot-gc-dev
mailing list