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.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
