RFR: 8296774: Removed default MEMFLAGS value from CHeapBitMap
Stefan Karlsson
stefank at openjdk.org
Thu Nov 10 09:33:58 UTC 2022
Today it is easy to accidentally create CHeapBitMaps that uses the default mtInternal MEMFLAGS instead of a value that is suitable for the subsystem. I fixed the instances I could find with #10948 / [JDK-8296231](https://bugs.openjdk.org/browse/JDK-8296231).
For that PR I didn't want to change the constructors of the bitmap because #10941 / [JDK-8296139](https://bugs.openjdk.org/browse/JDK-8296139) was being out for review. Now when that change has been pushed I'd like to change the constructors of the CHeapBitMap, so that we don't accidentally make these mistakes.
When making it mandatory to pass MEMFLAGS, it becomes apparent that the current parameter order is a bit odd. If you look closely you see that all three parameters are optional. When I now want to make MEMFLAGS mandatory, I'd like to move it so that it always is the first parameter. This will simplify the constructors a bit, IMHO.
This is what the constructors look like before the patch:
CHeapBitMap() : CHeapBitMap(mtInternal) {}
explicit CHeapBitMap(MEMFLAGS flags) : GrowableBitMap(0, false), _flags(flags) {}
CHeapBitMap(idx_t size_in_bits, MEMFLAGS flags = mtInternal, bool clear = true);
And I'd like to change it to:
explicit CHeapBitMap(MEMFLAGS flags) : GrowableBitMap(0, false), _flags(flags) {}
CHeapBitMap(MEMFLAGS flags, idx_t size_in_bits, bool clear = true);
In effect, this makes `flags` mandatory and `size_in_bits` and `clear` optional.
We could probably condense this even further into just one constructor:
explicit CHeapBitMap(MEMFLAGS flags, size_t size_in_bits = 0, bool clear = true) : GrowableBitMap(size_in_bits, clear), _flags(flags) {}
given that the value of `clear` doesn't matter when `size_in_bits` is 0. I didn't do that, but could be swayed to do that.
-------------
Commit messages:
- 8296774: Removed default MEMFLAGS value from CHeapBitMap
Changes: https://git.openjdk.org/jdk/pull/11084/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11084&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8296774
Stats: 20 lines in 13 files changed: 0 ins; 1 del; 19 mod
Patch: https://git.openjdk.org/jdk/pull/11084.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/11084/head:pull/11084
PR: https://git.openjdk.org/jdk/pull/11084
More information about the hotspot-dev
mailing list