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

The latest version looks good to me.


Marked as reviewed by iklam (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8156

Reply via email to