RFR: 8311648: Refactor the Arena/Chunk/ChunkPool interface [v6]

[Johan Sjölen]

On Wed, 19 Jul 2023 13:02:14 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Hi,
>> 
>> This PR refactors the Arena code without introducing any changes in behavior. I believe that this leads to a much clearer API and that this will in turn simplify the maintenance of this code. `Chunk` is now much more "dumb", mainly acting as a way of interrogating the chunk of data that it is holding. The `ChunkPool` gains a more prominent role, being entirely responsible for the memory allocation strategy used. `Arena` is basically unchanged, as it orchestrates the usage of these two APIs. The `chop` and `next_chop` methods are re-defined to be static inside of `Chunk`, having an object `delete` itself  is very surprising, so I'm happy to get rid of that.
>> 
>> I hope that you agree with me on these changes!
>> 
>> Cheers,
>> Johan
>
> Johan Sjölen has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 12 additional commits since the last revision:
> 
>  - Merge remote-tracking branch 'origin/master' into rework-arena
>  - Swap around argument
>  - Move stuff around to make diffs smaller
>  - Remove comments
>  - Hide ChunkPool, make ChunkPoolCleaner be started from Arena
>  - One more comment
>  - Remove comment sections
>  - Missed two small issues
>  - Move allocation responsibilities to ChunkPool
>  - Remove operator new/delete
>  - ... and 2 more: https://git.openjdk.org/jdk/compare/6f8b244a...d8196bab

Hi,

This PR introduces a bug in commit [Remove operator new/delete](https://github.com/openjdk/jdk/pull/14803/commits/3b2fc13382c75ce348453ff92ded4ad7549131e6) wherein a `ChunkPool` creates a circular linked list of chunks. No clue how that happens so far. Here's a gdb printout showing the phenomenom (breakpoint in `return_to_pool`.


(gdb) 
Continuing.
_first: (nil), chunk: 0x7ffff0171300, chunk->_next: (nil)
(gdb) 
Continuing.
_first: (nil), chunk: 0x7ffff0171300, chunk->_next: (nil)
(gdb) 
Continuing.
_first: 0x7ffff0171300, chunk: 0x7ffff002e680, chunk->_next: (nil)
(gdb) 
Continuing.
_first: 0x7ffff0171300, chunk: 0x7ffff002e680, chunk->_next: 0x7ffff0171300
(gdb) 
Continuing.
_first: 0x7ffff002e680, chunk: 0x7ffff0171300, chunk->_next: (nil)
(gdb) c
Continuing.
_first: 0x7ffff002e680, chunk: 0x7ffff0171300, chunk->_next: 0x7ffff002e680
(gdb) 
Continuing.
_first: 0x7ffff0171300, chunk: 0x7ffff002e680, chunk->_next: 0x7ffff0171300
(gdb) 
Continuing.
_first: 0x7ffff002e680, chunk: 0x7ffff0171300, chunk->_next: 0x7ffff002e680
(gdb) 
Continuing.
_first: 0x7ffff0171300, chunk: 0x7ffff002e680, chunk->_next: 0x7ffff0171300
(gdb) 
Continuing.
_first: 0x7ffff002e680, chunk: 0x7ffff0171300, chunk->_next: 0x7ffff002e680

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

PR Comment: https://git.openjdk.org/jdk/pull/14803#issuecomment-1668026624