RFR: 8296774: Removed default MEMFLAGS value from CHeapBitMap [v2]

Stefan Karlsson stefank at openjdk.org
Thu Nov 10 10:22:45 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.

Stefan Karlsson 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 two additional commits since the last revision:

 - Merge remote-tracking branch 'upstream/master' into 8296774_bitmap_stricter_construction
 - 8296774: Removed default MEMFLAGS value from CHeapBitMap

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/11084/files
  - new: https://git.openjdk.org/jdk/pull/11084/files/d83e5149..d1a3069b

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=11084&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11084&range=00-01

  Stats: 506 lines in 165 files changed: 84 ins; 53 del; 369 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