FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix AsciiString.cached(String) performance regression (#17074) by normanmaurer · Pull Request #17081 · netty/netty · GitHub

/ netty Public

Fix AsciiString.cached(String) performance regression (#17074) - #17081

Merged
normanmaurer merged 1 commit into
5.0from
ascii_perf5
Jul 10, 2026
Merged

normanmaurer merged 1 commit into
5.0from
ascii_perf5

Conversation

Copy link
Copy Markdown
Member

Motivation:

#17007 fixed #13749 by making
AsciiString.cached(String) sanitize non-Latin-1 input before caching
the String value.

That change preserved the correct behavior, but introduced a performance
regression. The implementation now performs two separate scans over the
input string:

  1. new AsciiString(string) copies chars into the backing byte array
    via c2b(...).
  2. AsciiString.cached(String) scans the same string again to check
    whether any char is greater than MAX_CHAR_VALUE.

PrintAssembly results show that C2 does not merge these two loops after
JIT compilation. The old version still contains the second scan loop in
AsciiString.cached, while the optimized version has a single loop that
performs both c2b(...) conversion and the Latin-1 check.

jit-actual-old-c2-ascii-final.txt

jit-actual-current-c2-ascii-final.txt

Modification:

Merge the byte conversion and Latin-1 detection into a single loop in
AsciiString.cached(String).

The method now allocates the byte array directly, fills it using the
existing c2b(char) conversion, and tracks whether every input char is
Latin-1 during the same pass. It still preserves the original String
instance for ASCII/Latin-1 input, and reconstructs the cached string
from the byte array only when sanitization is required.

Result:
This removes the extra input scan while preserving the behavior
introduced by #17007.

Motivation:

#17007 fixed #13749 by making
`AsciiString.cached(String)` sanitize non-Latin-1 input before caching
the `String` value.

That change preserved the correct behavior, but introduced a performance
regression. The implementation now performs two separate scans over the
input string:

1. `new AsciiString(string)` copies chars into the backing byte array
via `c2b(...)`.
2. `AsciiString.cached(String)` scans the same string again to check
whether any char is greater than `MAX_CHAR_VALUE`.

PrintAssembly results show that C2 does not merge these two loops after
JIT compilation. The old version still contains the second scan loop in
`AsciiString.cached`, while the optimized version has a single loop that
performs both `c2b(...)` conversion and the Latin-1 check.


[jit-actual-old-c2-ascii-final.txt](https://github.com/user-attachments/files/29848724/jit-actual-old-c2-ascii-final.txt)


[jit-actual-current-c2-ascii-final.txt](https://github.com/user-attachments/files/29848711/jit-actual-new-c2-ascii-final.txt)

Modification:

Merge the byte conversion and Latin-1 detection into a single loop in
`AsciiString.cached(String)`.

The method now allocates the byte array directly, fills it using the
existing `c2b(char)` conversion, and tracks whether every input char is
Latin-1 during the same pass. It still preserves the original `String`
instance for ASCII/Latin-1 input, and reconstructs the cached string
from the byte array only when sanitization is required.


Result:
This removes the extra input scan while preserving the behavior
introduced by #17007.
normanmaurer merged commit c44f90c into 5.0 Jul 10, 2026
12 of 13 checks passed
normanmaurer deleted the ascii_perf5 branch July 10, 2026 03:39
normanmaurer added this to the 5.0.0.Final milestone Jul 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL