On Thu, 7 Apr 2022 18:28:42 GMT, Roman Kennke <rken...@openjdk.org> wrote:
>> JVMTI heap walking marks objects in order to track which have been visited >> already. In order to do that, it uses bits in the object header. Those are >> the same bits that are also used by some GCs to mark objects (the lowest two >> bits, also used by locking code). Some GCs also use the bits in order to >> indicate 'forwarded' objects, where the upper bits of the header represent >> the forward-pointer. In the case of Shenandoah, it's even more problematic >> because this happens concurrently, even while JVMTI heap walks can >> intercept. So far we carefully worked around that problem, but it becomes >> very problematic in Lilliput, where accesses to the Klass* also requires to >> decode the header, and figure out what bits means what. >> >> In addition to that, marking objects in their header requires that the >> original header gets saved and restored. We only do that for 'interesting' >> headers, that is headers that have a stack-lock, monitor or hash-code. All >> other headers are reset to their default value. This means we are losing >> object's GC age. This is not catastrophic, but nontheless interferes with >> GC. >> >> JFR already has a datastructure called BitSet to support object marking >> without messing with object's headers. We can use that in JVMTI too. >> >> Testing: >> - [x] tier1 >> - [x] tier2 >> - [x] tier3 >> - [x] serviceability/jvmti >> - [x] vmTestbase/nsk/jvmti > > Roman Kennke has updated the pull request incrementally with one additional > commit since the last revision: > > Move JFRBitSet typedef into its own header; Make _bitset a direct member, > not dynamically allocated Hi Thomas, > > A general thought, maybe for a future RFE: > > We now have BitMap and BitSet, which do almost the same thing. Instead of > > having a new class BitSet, whose job is to be sparse, we could give BitMap > > the ability to be sparse. We'd save code, reuse the rich BitMap interface > > and get all the test code for free. > > One way to do that would be to make BitMap responsible for committing its > > underlying backing memory, in chunks, on-demand only. To do that BitMap > > would need a secondary map - a commit map - to remember which fragments of > > its backing memory are committed. Commit map would have a bit per 64M > > fragment. > > Please don't do this. `BitMap` (well, `BitMapView`) is also used as a > lightweight way to overlay it on arbitrary memory temporarily (i.e. > remembered sets bitmaps). This typically boils down to no additional memory > usage (and overhead) at all. So any additional memory usage or initialization > should not be added lightly. Also in most other cases, `BitMap` is used for > very tiny bitmaps anyway, this would just add unnecessary functionality (and > overhead) for 99% of the usage of `BitMap`. > > Further GCs are using `BitMap` to implement such sparse bit sets already, > managing their memory with the corresponding Java heap memory; so adding this > functionality at the `BitMap` level would just duplicate functionality with > probably not-so-hilarious side effects. > Yes, you are right. I had some more time to think about this and came to the same conclusion. Also abstracting bitwise getters and setters as I thought originally would probably not be enough. > I also _do_ think that the use cases for a dense `BitMap` are significantly > different from (huge) sparse "BitMap"s to warrant its own separate class (not > saying that one couldn't use the other internally, or that if that `BitSet` > could be reused in GCs if configurable enough). > > I am also not sure that the automatic lazy backing of the OS isn't sufficient > for these use cases (e.g. JVMTI) too. I'm not sure about this. We often run against the commit charge of the underlying OS, which seems to be more frequent than running out of physical memory. I think voluntary uncommit does make sense. Whether the libc actually uncommits on free() is a different question, but in this case, I think they do, the fragments are large enough. Thanks, Thomas ------------- PR: https://git.openjdk.java.net/jdk/pull/7964
