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

Mon Feb 13 19:29:53 UTC 2017

On 2/13/2017 11:21 AM, huizhe wang wrote:
> Hi Christoph,
>
> The negatives were actually across all platforms, although the 
> benchmark system picked up that it thinks were significant (high 
> probability of a regression). There were negatives numbers that were 
> bigger than the two.
>
> One possibility, if you look at the code change, is that the tests 
> contained cases where both uri and localName are null, with this 
> change then, there will be extra operations.

Oops, I meant to say the uri == null, but localName != null.

-Joe

>
> Thanks,
> Joe
>
> On 2/13/2017 7:29 AM, Langer, Christoph wrote:
>> 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.wang at oracle.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 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
>>>>>>>>>
>