RFR: 8285081: Improve XPath operators count accuracy
Adjust how XPath operators are counted to improve accuracy. This change does not affect how XPath works. Test: Tier2 passed; JCK XML tests passed. ------------- Commit messages: - 8285081: Improve XPath operators count accuracy Changes: https://git.openjdk.java.net/jdk/pull/9022/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=9022&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285081 Stats: 66 lines in 3 files changed: 43 ins; 3 del; 20 mod Patch: https://git.openjdk.java.net/jdk/pull/9022.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9022/head:pull/9022 PR: https://git.openjdk.java.net/jdk/pull/9022
On Fri, 3 Jun 2022 18:17:55 GMT, Joe Wang <joehw@openjdk.org> wrote:
Adjust how XPath operators are counted to improve accuracy. This change does not affect how XPath works.
Test: Tier2 passed; JCK XML tests passed.
src/java.xml/share/classes/com/sun/java_cup/internal/runtime/lr_parser.java line 152:
150: private int opCount = 0; 151: private int totalOpCount = 0; 152: private int lastSym;
`lastSym` is never initialized. It's OK for the first time, but should this be reset for the next use (if any), e.g. somewhere around line 595? src/java.xml/share/classes/com/sun/org/apache/xalan/internal/xsltc/compiler/sym.java line 99:
97: public static final int[] OPERATORS = {GT, GE, EQ, NE, LT, LE, SLASH, DSLASH, 98: DOT, DDOT, ATSIGN, DCOLON, PLUS, MINUS, STAR, DIV, MOD, AND, OR, LPAREN, 99: LBRACK, VBAR, DOLLAR, NODE, TEXT, PI, PIPARAM};
Any reason for re-shuffling the order of operators? I'd expect new ones are appended to the existing ones, or appear in the order of their declarations above? (or is this automatically generated, as described in the comment?) src/java.xml/share/classes/com/sun/org/apache/xpath/internal/compiler/Lexer.java line 351:
349: { 350: nesting++; 351: if (!isLiteral) {
`if (isLiteral) {` might be more readable. src/java.xml/share/classes/com/sun/org/apache/xpath/internal/compiler/Lexer.java line 377:
375: else if ((Token.LPAREN != c) && (Token.LBRACK != c) && (Token.RPAREN != c) 376: && (Token.RBRACK != c) && (Token.COLON != c) && (Token.COMMA != c)) { 377: if (Token.STAR == c) {
Could be if (Token.STAR != c || !isAxis) { incrementCount(c); } src/java.xml/share/classes/com/sun/org/apache/xpath/internal/compiler/Lexer.java line 476:
474: } 475: 476: private void incrementCount(char c) {
`c` is not used. ------------- PR: https://git.openjdk.java.net/jdk/pull/9022
On Fri, 3 Jun 2022 21:08:04 GMT, Naoto Sato <naoto@openjdk.org> wrote:
Adjust how XPath operators are counted to improve accuracy. This change does not affect how XPath works.
Test: Tier2 passed; JCK XML tests passed.
src/java.xml/share/classes/com/sun/java_cup/internal/runtime/lr_parser.java line 152:
150: private int opCount = 0; 151: private int totalOpCount = 0; 152: private int lastSym;
`lastSym` is never initialized. It's OK for the first time, but should this be reset for the next use (if any), e.g. somewhere around line 595?
Right, -1 would be appropriate as 0 indicates EOF. ------------- PR: https://git.openjdk.java.net/jdk/pull/9022
On Fri, 3 Jun 2022 21:11:20 GMT, Naoto Sato <naoto@openjdk.org> wrote:
Adjust how XPath operators are counted to improve accuracy. This change does not affect how XPath works.
Test: Tier2 passed; JCK XML tests passed.
src/java.xml/share/classes/com/sun/org/apache/xalan/internal/xsltc/compiler/sym.java line 99:
97: public static final int[] OPERATORS = {GT, GE, EQ, NE, LT, LE, SLASH, DSLASH, 98: DOT, DDOT, ATSIGN, DCOLON, PLUS, MINUS, STAR, DIV, MOD, AND, OR, LPAREN, 99: LBRACK, VBAR, DOLLAR, NODE, TEXT, PI, PIPARAM};
Any reason for re-shuffling the order of operators? I'd expect new ones are appended to the existing ones, or appear in the order of their declarations above? (or is this automatically generated, as described in the comment?)
The order is not significant for this process as the lexer takes care of creating the right symbol. I re-grouped them to put operators of the same category together so that it's easier to see what might be missing.
src/java.xml/share/classes/com/sun/org/apache/xpath/internal/compiler/Lexer.java line 476:
474: } 475: 476: private void incrementCount(char c) {
`c` is not used.
Will remove it. ------------- PR: https://git.openjdk.java.net/jdk/pull/9022
On Fri, 3 Jun 2022 21:52:08 GMT, Naoto Sato <naoto@openjdk.org> wrote:
Joe Wang has updated the pull request incrementally with one additional commit since the last revision:
review update
src/java.xml/share/classes/com/sun/org/apache/xpath/internal/compiler/Lexer.java line 377:
375: else if ((Token.LPAREN != c) && (Token.LBRACK != c) && (Token.RPAREN != c) 376: && (Token.RBRACK != c) && (Token.COLON != c) && (Token.COMMA != c)) { 377: if (Token.STAR == c) {
Could be
if (Token.STAR != c || !isAxis) { incrementCount(c); }
Thanks Naoto! Updated as commented. ------------- PR: https://git.openjdk.java.net/jdk/pull/9022
Adjust how XPath operators are counted to improve accuracy. This change does not affect how XPath works.
Test: Tier2 passed; JCK XML tests passed.
Joe Wang has updated the pull request incrementally with one additional commit since the last revision: review update ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/9022/files - new: https://git.openjdk.java.net/jdk/pull/9022/files/f840e840..e1e3deb2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=9022&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=9022&range=00-01 Stats: 13 lines in 2 files changed: 4 ins; 7 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/9022.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9022/head:pull/9022 PR: https://git.openjdk.java.net/jdk/pull/9022
Adjust how XPath operators are counted to improve accuracy. This change does not affect how XPath works.
Test: Tier2 passed; JCK XML tests passed.
Joe Wang has updated the pull request incrementally with one additional commit since the last revision: remove unused parameter ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/9022/files - new: https://git.openjdk.java.net/jdk/pull/9022/files/e1e3deb2..6dc1cc2c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=9022&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=9022&range=01-02 Stats: 9 lines in 1 file changed: 0 ins; 0 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/9022.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9022/head:pull/9022 PR: https://git.openjdk.java.net/jdk/pull/9022
On Fri, 3 Jun 2022 23:56:31 GMT, Joe Wang <joehw@openjdk.org> wrote:
Adjust how XPath operators are counted to improve accuracy. This change does not affect how XPath works.
Test: Tier2 passed; JCK XML tests passed.
Joe Wang has updated the pull request incrementally with one additional commit since the last revision:
remove unused parameter
LGTM ------------- Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/9022
On Fri, 3 Jun 2022 23:56:31 GMT, Joe Wang <joehw@openjdk.org> wrote:
Adjust how XPath operators are counted to improve accuracy. This change does not affect how XPath works.
Test: Tier2 passed; JCK XML tests passed.
Joe Wang has updated the pull request incrementally with one additional commit since the last revision:
remove unused parameter
Marked as reviewed by lancea (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/9022
On Fri, 3 Jun 2022 18:17:55 GMT, Joe Wang <joehw@openjdk.org> wrote:
Adjust how XPath operators are counted to improve accuracy. This change does not affect how XPath works.
Test: Tier2 passed; JCK XML tests passed.
This pull request has now been integrated. Changeset: 8e078391 Author: Joe Wang <joehw@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/8e0783917975075aae5d586f0076d5093afb... Stats: 63 lines in 3 files changed: 40 ins; 3 del; 20 mod 8285081: Improve XPath operators count accuracy Reviewed-by: naoto, lancea ------------- PR: https://git.openjdk.java.net/jdk/pull/9022
participants (3)
-
Joe Wang
-
Lance Andersen
-
Naoto Sato