8280124: Reduce branches decoding latin-1 chars from UTF-8 encoded bytes

[cl4es]

This resolves minor inefficiency in the fast-path for decoding latin-1 chars from UTF-8. I also took the opportunity to refactor the StringDecode microbenchmark to align with recent changes to the StringEncode micro.

The inefficiency is that this test is quite branchy:

if ((b1 == (byte)0xc2 || b1 == (byte)0xc3) && ...

Since the two constant bytes differ only on the lowest bit this can be transformed to this, saving us a branch:

if ((b1 & 0xfe) == 0xc2 && ...

This provides a small speed-up on microbenchmarks where the input can be internally encoded as latin1:

Benchmark (charsetName) Mode Cnt Score Error Units
StringDecode.decodeLatin1LongStart UTF-8 avgt 50 2283.591 ± 12.332 ns/op

StringDecode.decodeLatin1LongStart UTF-8 avgt 50 2165.984 ± 13.136 ns/op 

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8280124: Reduce branches decoding latin-1 chars from UTF-8 encoded bytes

Reviewers

• Roger Riggs (@RogerRiggs - Reviewer) ⚠️ Review applies to 926c32b
• Alan Bateman (@AlanBateman - Reviewer) ⚠️ Review applies to 926c32b
• Naoto Sato (@naotoj - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/7122/head:pull/7122
$ git checkout pull/7122

Update a local copy of the PR:
$ git checkout pull/7122
$ git pull https://git.openjdk.java.net/jdk pull/7122/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 7122

View PR using the GUI difftool:
$ git pr show -t 7122

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/7122.diff

[bridgekeeper]

👋 Welcome back redestad! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@cl4es The following label will be automatically applied to this pull request:

• core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 01: Full - Incremental (e4b6c40a)
• 00: Full (926c32bf)

[cl4es]

On a microbenchmark that zooms in on the logical predicate the speed-up is closer to 2x. This seems like a transformation a JIT could do automatically. gcc and clang doesn't do it, but icc seem to pull it off (as tested via godbolt.org). It's unclear if this is common enough to motivate such enhancement work, but it might be of academic interest to attempt it.

[RogerRiggs]

LGTM

[openjdk]

@cl4es This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8280124: Reduce branches decoding latin-1 chars from UTF-8 encoded bytes

Reviewed-by: rriggs, alanb, naoto

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 28 new commits pushed to the master branch:

• 7acc4c7: 8280058: JFR: StreamUtils::getJfrRepository(Process) should print stdout and stderr
• 88a8b23: 8280076: Unify IGV and IR printing
• 9e3f68d: 8279290: symbol not found error, implicit lambdas and diamond constructor invocations
• 9eb50a5: 8280010: Remove double buffering of InputStream for Properties.load
• 64c0c0e: 8276563: Undefined Behaviour in class Assembler
• d175d33: 8280079: Serial: Remove empty Generation::prepare_for_verify
• 1725f77: 8280029: G1: "Overflow during reference processing, can not continue" on x86_32
• 645b38d: 8280089: compiler/c2/irTests/TestIRAbs.java fails on some arches
• eb94995: 8280070: G1: Fix template parameters in G1SegmentedArraySegment
• 9452262: 8278892: java.naming module description is missing @uses tags to document the services that it uses
• ... and 18 more: https://git.openjdk.java.net/jdk/compare/9e536b64705f841b224d0e64cad0f1609ebf5bca...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[cl4es]

Thanks for reviewing, everyone!

/integrate

[openjdk]

Going to push as commit e314a4c.
Since your change was applied there have been 32 commits pushed to the master branch:

• bdfa15d: 8250801: Add clhsdb "threadcontext" command
• fd9fb9a: 8279194: Add Annotated Memory Viewer feature to SA's HSDB
• 848b16a: 8272746: ZipFile can't open big file (NegativeArraySizeException)
• b734dc8: 8280055: JFR: Improve ObjectContext implementation
• 7acc4c7: 8280058: JFR: StreamUtils::getJfrRepository(Process) should print stdout and stderr
• 88a8b23: 8280076: Unify IGV and IR printing
• 9e3f68d: 8279290: symbol not found error, implicit lambdas and diamond constructor invocations
• 9eb50a5: 8280010: Remove double buffering of InputStream for Properties.load
• 64c0c0e: 8276563: Undefined Behaviour in class Assembler
• d175d33: 8280079: Serial: Remove empty Generation::prepare_for_verify
• ... and 22 more: https://git.openjdk.java.net/jdk/compare/9e536b64705f841b224d0e64cad0f1609ebf5bca...master

Your commit was automatically rebased without conflicts.

[openjdk]

@cl4es Pushed as commit e314a4c.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

[mlbridge]

Mailing list message from John Rose on core-libs-dev:

It?s kind of sad that you have to do this by hand.
The JVM should recognize that those back-to-back
if statements (in the bytecode) can be combined
into a non-branching O(1) test.

One item on my (very long) wish list for the JVM
is giving more comprehensive attention to the task
of testing membership in small finite sets. This
one asks whether a given byte is one of two values,
where they happen to be consecutive and also have
almost the same bit pattern. A more general version
of the problem would be any switch of this form:

switch (x) {
case 2: case 3: case 5: case 7: case 16:
default: return false;
}

As you can see, this is again a test for membership
in a small, statically determined set. As long as
the static set is ?small enough? (a flexible definition)
there is always a way that is ?very fast? (flexible again)
to detect set membership, and in particular faster than
a branch tree (unless you know which branch is very likely
to be taken). The ?common bad? case is having to generate
a small perfect hash function, based on a multiply. The
best case is an ad hoc detection of an arithmetic or
bitwise pattern, as you have done here by hand.

If the JVM had such a set-test generator, it could (a) turn
branch nests (from switches or from random user code) into
O(1) set tests, and (b) also supply a method handle factory
for them, which would be a very useful way to translate
indy-mediated discrimination trees that underly
pattern-matching switches. Also it could translate
character class tests in the regex package. (I started
thinking about this long ago when staring at hand-written
switches in Java code that implement character classes.)

? John

On 18 Jan 2022, at 2:44, Claes Redestad wrote: