RFR: JDK-8272112: Arena code simplifications [v3]
Thomas Stuefe
stuefe at openjdk.java.net
Fri Aug 13 04:06:41 UTC 2021
> May I please have reviews for this small change. This is pure code cleanup, issues that were identified by code review when working on JDK-8270308.
>
> The patch results in less code (excluding the new gtest), bit easier to maintain. I have some smaller functional improvements in the pipeline and would like to get this code cleanup out of the way first.
>
> Changes:
>
> - Made `ChunkPool` an automatic object. We can remove `chunkpool_init()` and instead rely on automatic initialization.
>
> - Instead of having four named pools that need to be handled individually and selected via awkward switch constructs (see the old `Chunk::operator new()` and `::operator delete()`), we replace it with an array and a static selector (`ChunkPool* get_pool_for_size(size_t size)`).
>
> - Removed `ChunkPool::_num_used`. It was nowhere queried, and its usefulness was questionable since these numbers don't include chunks with "special" sizes. The easiest way to obtain the footprint of arena chunks is via NMT.
>
> - Before, we had two places where the backing memory of Chunks was allocated: in `ChunkPool::allocate()` and in `Chunk::operator new()`. The former was unnecessary. I streamlined that: removed the ability to allocate from `ChunkPool::allocate()` and renamed it to `ChunkPool::remove_chunk()`. Now Chunks are only allocated in `Chunk::operator new() `and ChunkPool just serves as Chunk cache.
>
> - Renamed `ChunkPool::free_all_but()` to `ChunkPool::prune()` and removed the argument, made it a method-local constant (it had been a constant before in the caller of that function).
>
> - I added a new gtest to test Chunk allocation and pooling a little.
>
>
> Functionally, nothing should have changed with this patch, this is pure code grooming.
>
> ----
>
> Tests:
> - GHA
> - I manually tested on 64-bit and 32-bit, with and without UseMallocOnly. I also manually checked that pool usage is as expected.
Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
feedback coleen
-------------
Changes:
- all: https://git.openjdk.java.net/jdk/pull/5040/files
- new: https://git.openjdk.java.net/jdk/pull/5040/files/625df135..52295624
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5040&range=02
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5040&range=01-02
Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
Patch: https://git.openjdk.java.net/jdk/pull/5040.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5040/head:pull/5040
PR: https://git.openjdk.java.net/jdk/pull/5040
More information about the hotspot-dev
mailing list