RFR: JDK-8272112: Arena code simplifications

Kim Barrett kbarrett at openjdk.java.net
Mon Aug 9 08:40:35 UTC 2021


On Mon, 9 Aug 2021 08:14:20 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>     * `ChunkPool::prune()` will not work with `blocksToKeep=0` (never did). I guess that is okay.

Yeah, that seems fine now that it's a constant.  If you want to be paranoid and add a comment or even a static_assert, that would be fine with me, but not necessary.

>     * I thought about factoring out "free chunk and followers" as a new `ChunkPool::free_list()` from `ChunkPool::prune()` because strictly speaking we should do this also at destruction. But the only instances of `ChunkPool` are globals, and there is no need to spend time on cleanup when the VM exits. Is there a pattern somehow to mark a class as "its okay to leak resources since we only use this class as static globals"?

I don't know of any idiom in HotSpot for tagging such places.  Add a comment if you feel the need to do so?

>     * The actual allocation of backing memory for chunks is spread over various places (`Chunk::operator new()`, `Chunk::operator delete()`, `ChunkPool::prune()`). I wondered if for clarity we should factor them out into clearly marked functions. I did not do that to keep the patch small.

It's not obvious that's worth the extra work.  It seems fine as is to me.

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

PR: https://git.openjdk.java.net/jdk/pull/5040


More information about the hotspot-dev mailing list