RFR: JDK-8307356: Metaspace: simplify BinList handling
Roman Kennke
rkennke at openjdk.org
Thu May 4 11:48:18 UTC 2023
On Wed, 3 May 2023 12:24:48 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
> In preparation for Lilliput, I'd like to simplify BinList handling a bit.
>
> BinList are a data structure that stores small blocks that had been prematurely deallocated for re-use. Due to the way BinList is implemented we need the blocks to be at least two words. That causes ripples all across metaspace, including testing code. We can get rid of this complexity by shrinking a Block to just one word.
>
> ----
>
> Details:
>
> A BinList block before:
> ``` Block { Block* next; size_t size; }```
> A BinList block now:
> ``` Block { Block* next; }```
>
> We don't need to store the block size, since all blocks in a single sub list have the same size. In fact, we used the block size only for verification.
>
> That means that for 64-bit gross- and net-allocation size in Metaspace are the same (see `get_raw_word_size_for_requested_word_size`), which in turn will make writing tests that try to predict metaspace usage based on allocation pattern a lot less onerous to write.
Looks mostly ok. I'd keep MinWordSize constants, though.
src/hotspot/share/memory/metaspace/binList.hpp line 98:
> 96:
> 97: // Minimal word size a block must have to be manageable by this structure.
> 98: const static size_t MinWordSize = smallest_word_size;
Maybe keep MinWordSize = 1, then we can also keep using it, e.g. in code below:
int index = (int)(word_size - MinWordSize);
and many other places. Otherwise I wouldn't know what that '1' means.
src/hotspot/share/memory/metaspace/freeBlocks.cpp line 56:
> 54: // the remainder is handed back to the manager.
> 55: const size_t waste = real_size - requested_word_size;
> 56: if (waste > MinWordSize) {
Has the calculation been wrong before or should that be > 1 or am I missing something?
src/hotspot/share/memory/metaspace/metaspaceArena.cpp line 63:
> 61: assert_lock_strong(lock());
> 62: size_t remaining_words = c->free_below_committed_words();
> 63: if (remaining_words > 0) {
Same here.
-------------
Changes requested by rkennke (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/13776#pullrequestreview-1412908040
PR Review Comment: https://git.openjdk.org/jdk/pull/13776#discussion_r1184899152
PR Review Comment: https://git.openjdk.org/jdk/pull/13776#discussion_r1184901547
PR Review Comment: https://git.openjdk.org/jdk/pull/13776#discussion_r1184901913
More information about the hotspot-runtime-dev
mailing list