RFR: 8263790: C2: new igv_print_immediately() for debugging purpose

Christian Hagedorn chagedorn at openjdk.java.net
Fri Mar 19 10:44:42 UTC 2021


On Fri, 19 Mar 2021 09:09:08 GMT, Yi Yang <yyang at openjdk.org> wrote:

>> Hi @kelthuzadx, I cannot reproduce the problem running the latest IGV on JDK 8 and linux-x64 either.
>> 
>> Note that IGV is designed to tolerate XML files without closing `</graphDocument>` and `</group>` tags. The parser does not use an error handler, which leads to ignoring all parsing errors [1], and on top of that certain parsing exceptions are ignored:
>> 
>> https://github.com/openjdk/jdk/blob/d24e4cfef36026b781906a9e0c5cf519eb72696e/src/utils/IdealGraphVisualizer/Data/src/com/sun/hotspot/igv/data/serialization/Parser.java#L531-L539
>> 
>> [1] https://docs.oracle.com/javase/8/docs/api/org/xml/sax/XMLReader.html#setErrorHandler-org.xml.sax.ErrorHandler-
>> 
>> Perhaps the problem you report should be addressed by making sure IGV also accepts missing closing `</graphDocument>` and `</group>` tags in your case? As you mention, this is a common debugging scenario.
>
>> Perhaps the problem you report should be addressed by making sure IGV also accepts missing closing </graphDocument> and </group> tags in your case?
> 
> Yes, absolutely.
> 
> Thank you all for pointing out this. It seems that my IGV is too old to parse incomplete xml documents:-( . I will update it to let it work.
> 
> The original intention of this patch is to generate complete xml, but it seems that even incomplete xml can be opened by the latest IGV, so this patch seems to be of little significance(Maybe the only use is to make the old version IGV happy).
> 
> Thanks!
> Yang

By not adding the closing tags to the xml file after `igv_print()`, subsequent calls can immediately append a graph to the current method which makes it easier to look at them in the IGV. I think that is wanted most of the time. If not then we need something like in your patch. However, if I see this correctly, the `Debug.xml` file in your patch would be overridden each time calling `igv_print_immediately()`, only allowing to have one graph at a time which is probably not desired. But either way, I'm not sure if it is worth offering such flexibility.

> I will update it to let it work.

Sounds good.

-------------

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


More information about the hotspot-compiler-dev mailing list