> Hi all, > > can I have reviews for this change that adds dedicated filler objects to > the VM? > > Currently, when formatting areas of dead objects all gcs use instances of > j.l.Object and int-arrays. > > This has the drawback of not being easily able to discern whether a given > object is actually dead (and should never be referenced) or just a regular > j.l.Object/int array. > > This also makes enhanced error detection (any reference to such an object is > an error - i.e. detecting references to such objects) and to skip potentially > already unloaded classes when scanning areas of the heap below TAMS, G1 uses > its prev bitmap. > Other collectors do not have this extra information at the moment, so they > can't (and don't) do this kind of verification. > > With [JDK-8210708](https://bugs.openjdk.java.net/browse/JDK-8210708) the prev > bitmap will effectively be removed in G1; G1 will format the dead areas with > these filler objects to avoid coming across unloaded classes. This is fine > wrt to normal operation, however, this looses the existing enhanced > verification mentioned above. > > This change proposes to add dedicated VM-internal filler objects, i.e. > equivalents of j.l.Object and int-arrays. > > This has the following benefits: > > - keep this error detection (actually making it much simpler) and allowing > similar verification for other collectors. (This change does not add this) > > - this also makes it "easy" to detect references to filler objects in > debugging tools - you only need to know the two klasses (or just get their > friendly name) to see whether that reference may actually be valid (or refers > to the inside such an object). References to these classes in the crash file > may also allow the issue to be more clear. > > This causes some minor changes to external behavior: > > - logs/heap dumps now contain instances of these objects - which seems fine > as previously they have just been reported as part of j.l.Object/int-arrays > statistics. The VM spec also does not guarantee whether a particular kind of > object should/should not show there anyway afaik. > > - if the application ever gets to instantiate a reference to such an object > somehow, any enabled verification will crash the VM. That's bad luck for > messing with internal classes, but that's the purpose of these objects. > > The change takes care that getting a reference will not be possible by normal > means (i.e. via Class.forName() etc) - which should be sufficient to avoid > the issue. Actually, existing mechanisms seem to be sufficient. > > > Testing: tier1-8 > > There is one question I would like the reviewers to specially think about, > the name of the filler array klass. I just used > `Ljava/internal/vm/FillerArray;` for that, looking at other internal > symbols/klasses, but I'm not sure this adheres to naming guidelines. > > Thanks go to @iklam for helping out with the change. > > Thanks, > Thomas
Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision: Fix test ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/8156/files - new: https://git.openjdk.java.net/jdk/pull/8156/files/b3e2c6c3..effb9cd5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8156&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8156&range=01-02 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8156.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8156/head:pull/8156 PR: https://git.openjdk.java.net/jdk/pull/8156