RFR: 8132995: Matcher$ImmutableMatchResult should be optimized to reduce space usage [v2]

Claes Redestad redestad at openjdk.org
Tue May 16 13:42:59 UTC 2023


On Thu, 11 May 2023 09:22:44 GMT, Raffaello Giulietti <rgiulietti at openjdk.org> wrote:

>> When appropriate and useful, copies only the relevant portion of the `CharSequence` to the match result.
>
> Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Added some randomness in tests.

Overall looks ok but the `captureText` usage in `Matcher::results` seem suspect.

src/java.base/share/classes/java/util/regex/Matcher.java line 359:

> 357:             if ((groups[group * 2] == -1) || (groups[group * 2 + 1] == -1))
> 358:                 return null;
> 359:             return text.subSequence(groups[group * 2] - offset, groups[group * 2 + 1] - offset).toString();

Could be simplified to `return text.substring(groups[group * 2] - offset, groups[group * 2 + 1] - offset);`

src/java.base/share/classes/java/util/regex/Matcher.java line 1381:

> 1379: 
> 1380:                 // Capture the input sequence as a string on first find
> 1381:                 textAsString = captureText();

Is this correct? The `find()` on line 1371 will set `first` and `last` and `captureText` uses that to narrow down `textAsString` to that range.. so the next `find()` will then find a new `first` and `last` pair but use the text captured after the first `find`?

-------------

PR Review: https://git.openjdk.org/jdk/pull/13231#pullrequestreview-1428589260
PR Review Comment: https://git.openjdk.org/jdk/pull/13231#discussion_r1195169128
PR Review Comment: https://git.openjdk.org/jdk/pull/13231#discussion_r1195181270


More information about the core-libs-dev mailing list