Hi Joe, this is an updated webrev with only the change in namespace handling and the testcase: http://cr.openjdk.java.net/~clanger/webrevs/8172974.1-SAX2DTM2only/
I'd really be interested in the performance results for that one before I turn to JDK10. Thanks Christoph > -----Original Message----- > From: Langer, Christoph > Sent: Samstag, 11. Februar 2017 15:50 > To: 'huizhe wang' <huizhe.w...@oracle.com> > Cc: core-libs-dev@openjdk.java.net > Subject: RE: Update: RFR 8172974 [JAXP] XALAN: Wrong result when > transforming namespace unaware StAX Input > > 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.w...@oracle.com] > > Sent: Donnerstag, 9. Februar 2017 23:15 > > To: Langer, Christoph <christoph.lan...@sap.com> > > Cc: core-libs-dev@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.w...@oracle.com>; > > >>> core-libs-dev@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.w...@oracle.com] > > >>>> Sent: Mittwoch, 18. Januar 2017 22:12 > > >>>> To: Langer, Christoph<christoph.lan...@sap.com>; core-libs- > > >>>> d...@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 > > >>>>>
