On Mon, 9 Sep 2024 19:04:13 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> I pulled your changes and I see one slight difference in the output. The >> following line is missing: >> >> `_metadata._compressed_klass: InstanceKlass for >> java/util/concurrent/locks/AbstractQueuedSynchronizer$ConditionObject` >> >> I realize that there is no `_metadata._compressed_klass` when you have >> compact headers, and that the Klass* is encoded in the `_mark` word, which >> is now looks something like this in the output: >> >> _mark: 16294762323640321 >> >> So you can say that the Klass* is embedded in the _mark work, but this isn't >> of much help to SA users. I think what is expected is that the visitor is >> passed a MetadataField object that when getValue() is called on it, the >> Klass mirror is returned. Maybe we need a new CompactKlassField type like we >> current have a NarrowKlassField field type, and it will do the decoding of >> the _mark work into a Klass. The current getKlass() is related to this. > > Thinking about this a bit more, maybe _mark needs to be a MetadataFile rather > than CInt. This is a kind of odd situation. Basically we have a CInt field > that is more than just simple bits used as flags or small integers. It also > gets you to the Klass*. Possibly SA should treat _mark is two seprate fields; > one that remains a CInt as it currently is and another that treats it as an > encoded Klass* like the NarrowKlassField case. Do you think this needs to be addressed before integration? And if so, could you help with implementation? Or could we do it after intergration? Then please file a follow-up issue. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1764976086
