On Mon, 11 Apr 2022 14:55:32 GMT, Thomas Schatzl <tscha...@openjdk.org> wrote:
>> 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 Lgtm! ------------- Marked as reviewed by iwalulya (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8156