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