Update: RFR 8172974 [JAXP] XALAN: Wrong result when transforming namespace unaware StAX Input

[Langer, Christoph]

Hi Joe,

that's unfortunate but if these are the numbers then we have to deal with it. It's a bit odd, though, that this does not cause a regression on all platforms. So, you are really sure the regression roots in my current proposal for 8172974?

One thing I'd want to have been looked at is if the regression really roots to my change in SAX2DTM2.java. This is the real modification. Everything else is just cleanup. And looking at SAX2DTM2, I really can't see where I should have introduced something bad. Can you retest this?

Also, moving this to JDK10 would be fine for me. Shall I submit my fix there as is or do you want to conduct some deeper analysis about the performance regression's root cause? Can I somehow assist with that?

Thanks
Christoph

> -----Original Message-----
> From: huizhe wang [mailto:huizhe.wang at oracle.com]
> Sent: Donnerstag, 9. Februar 2017 23:15
> To: Langer, Christoph <christoph.langer at sap.com>
> Cc: core-libs-dev at openjdk.java.net
> Subject: Re: Update: RFR 8172974 [JAXP] XALAN: Wrong result when
> transforming namespace unaware StAX Input
> 
> Hi Christoph,
> 
> The performance results showed two regressions, one on Linux and another
> Solaris SPARC, while at the same time a bit more than half of the
> results across 5 platforms were negative. This is unfortunate. We're too
> close to the release date, it's super sensitive for jaxp on the
> performance front. I'd like to suggest that we move this patch to a
> 9-update or 10 when we'd have plenty of time to deal with any possible
> fallout. I consulted our performance expert and this is also his
> recommendation. I hope you'd understand.
> 
> Thanks,
> Joe
> 
> On 2/7/2017 4:54 PM, Joe Wang wrote:
> > Hi Christoph,
> >
> > We'll double-check whether there's any performance implications.
> > Please give us a couple of (or few) days.
> >
> > Thanks,
> > Joe
> >
> > On 1/30/17, 7:28 AM, Langer, Christoph wrote:
> >> Hi Joe,
> >>
> >> I've updated my webrev for 8172974 after pulling out the refactoring
> >> of javax/xml/jaxp/unittest/transform/TransformerTest.java to Bug
> >> 8173602: http://cr.openjdk.java.net/~clanger/webrevs/8172974.1/
> >>
> >> After exchanging some off-list mails with Lance and Daniel, we came
> >> to the conclusion that you should decide about this one when you are
> >> back. The changes are merely cleanups apart from
> >>
> com/sun/org/apache/xml/internal/dtm/ref/sax2dtm/SAX2DTM2.startEleme
> nt(),
> >> which to me seem natural after 8169631, though.
> >>
> >> Thanks,
> >> Christoph
> >>
> >>> -----Original Message-----
> >>> From: Langer, Christoph
> >>> Sent: Mittwoch, 18. Januar 2017 22:55
> >>> To: 'huizhe wang'<huizhe.wang at oracle.com>;
> >>> core-libs-dev at openjdk.java.net
> >>> Subject: RE: RFR 8172974 [JAXP] XALAN: Wrong result when
> transforming
> >>> namespace unaware StAX Input
> >>>
> >>> Hi Joe,
> >>>
> >>> generally, you are right, XSLT requires namespace support. For
> >>> parsing the
> >>> stylesheet, it is definitely a hard requirement. Otherwise this
> >>> would not make
> >>> sense at all. For instance xsl directives are in the xsl namespace.
> >>> That
> >>> requirement is what the FAQ you are referencing [1] is talking about.
> >>>
> >>> As for the InputSource which is to be processed, it is probably also
> >>> not a right to
> >>> use non namespace aware parsing. But still it's not forbidden. For
> >>> instance, the
> >>> JavaDoc for SAXSource [2] states this:
> >>> "Note that XSLT requires namespace support. Attempting to transform
> >>> an input
> >>> source that is not generated with a namespace-aware parser may
> >>> result in
> >>> errors. Parsers can be made namespace aware by calling the
> >>> SAXParserFactory.setNamespaceAware(boolean awareness) method."
> >>>
> >>> So, I agree, we are in the error space here. But still I think the
> >>> result of non
> >>> namespace aware parsing should be the same for all types of input
> >>> source. And
> >>> at the moment it is the same for DOMSource and SAXSource but not for
> >>> StAXSource. From that point of view I think my fix makes sense
> >>> (along with the
> >>> other cleanups).
> >>>
> >>> Best regards
> >>> Christoph
> >>>
> >>> [1] https://xml.apache.org/xalan-j/faq.html#faq-N10207
> >>> [2]
> >>>
> https://docs.oracle.com/javase/8/docs/api/javax/xml/transform/sax/SAXSo
> urce
> >>>
> >>> .html
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: huizhe wang [mailto:huizhe.wang at oracle.com]
> >>>> Sent: Mittwoch, 18. Januar 2017 22:12
> >>>> To: Langer, Christoph<christoph.langer at sap.com>; core-libs-
> >>>> dev at openjdk.java.net
> >>>> Subject: Re: RFR 8172974 [JAXP] XALAN: Wrong result when
> transforming
> >>>> namespace unaware StAX Input
> >>>>
> >>>> Hi Christoph,
> >>>>
> >>>> Xalan requires the underlying parser to be namespace aware. Please
> >>>> refer
> >>>> to https://xml.apache.org/xalan-j/faq.html#faq-N10207
> >>>>
> >>>> Thanks,
> >>>> Joe
> >>>>
> >>>> On 1/18/2017 8:26 AM, Langer, Christoph wrote:
> >>>>> Hi,
> >>>>>
> >>>>> please review a change for JAXP.
> >>>>>
> >>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8172974
> >>>>> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8172974.0/
> >>>>>
> >>>>> When enhancing the test for
> https://bugs.openjdk.java.net/browse/JDK-
> >>>> 8023653, I saw that there's still an issue with StAXInputSource
> >>>> which is not
> >>>> namespace aware. This needs a small update in
> >>>>
> >>>
> src/java.xml/share/classes/com/sun/org/apache/xml/internal/dtm/ref/sax2
> dt
> >>>
> >>>> m/SAX2DTM2.java. Furthermore, I added the fixed warnings and
> >>>> formattings
> >>>> from the proposal
> >>>> http://cr.openjdk.java.net/~clanger/webrevs/8023653.0/ to
> >>>> this webrev, as 8023653 is an enhancement and might not go in in
> >>>> the near
> >>>> future.
> >>>>> I also enhanced the TransformerTest to utilize data providers now
> >>>>> and test
> >>> a
> >>>> comprehensive matrix of XALAN input.
> >>>>> Thanks in advance and Best regards
> >>>>> Christoph
> >>>>>