Re: RFR: 8285081: Improve XPath operators count accuracy [v3]

2022-06-07 Thread Lance Andersen
On Fri, 3 Jun 2022 23:56:31 GMT, Joe Wang  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


Re: RFR: 8285081: Improve XPath operators count accuracy [v3]

2022-06-06 Thread Naoto Sato
On Fri, 3 Jun 2022 23:56:31 GMT, Joe Wang  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


Re: RFR: 8285081: Improve XPath operators count accuracy [v3]

2022-06-03 Thread Joe Wang
> 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=9022=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9022=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


Re: RFR: 8285081: Improve XPath operators count accuracy [v2]

2022-06-03 Thread Joe Wang
On Fri, 3 Jun 2022 21:52:08 GMT, Naoto Sato  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


Re: RFR: 8285081: Improve XPath operators count accuracy [v2]

2022-06-03 Thread Joe Wang
> 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=9022=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9022=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


Re: RFR: 8285081: Improve XPath operators count accuracy

2022-06-03 Thread Joe Wang
On Fri, 3 Jun 2022 21:11:20 GMT, Naoto Sato  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


Re: RFR: 8285081: Improve XPath operators count accuracy

2022-06-03 Thread Joe Wang
On Fri, 3 Jun 2022 21:08:04 GMT, Naoto Sato  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


Re: RFR: 8285081: Improve XPath operators count accuracy

2022-06-03 Thread Naoto Sato
On Fri, 3 Jun 2022 18:17:55 GMT, Joe Wang  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