On Sun, 3 Dec 2023 01:01:49 GMT, David Schlosnagle <[email protected]> wrote:
>> This improvement has been found on >> https://github.com/vert-x3/vertx-web/pull/2526. >> >> It can potentially affect the existing ArraysSupport.mismatch caller >> code-path performance ie requires investigation. > > src/java.base/share/classes/java/lang/String.java line 2184: > >> 2182: byte[] tv = value; >> 2183: byte[] ov = other.value; >> 2184: if (coder == otherCoder) { > > Is this change needed as `otherCoder` is not reused? > > Suggestion: > > byte[] tv = value; > byte[] ov = other.value; > byte coder = coder(); > if (coder == other.coder()) { Not necessary, but given that later, in the path I have introduced, I am using directly the coder byte to both compute the actual length of `tv` (inlining String::length) and verify the coder to be the same for both `tv` and `ov`, I have avoided fetching it twice. I see that "could" increase the register pressure, but I haven't verified it yet; there is any performance concern or is a readability/style-only concern? > Could we save a comparison by ORing offsets? I'm ok to use the or to save an additional offset comparison, good idea! > Should this also handle UTF-16? Currently String::equals doesn't use the UTF16 variant of the intrinsic, see https://github.com/openjdk/jdk/blob/949846986f572dfb82912e7d71e7bfd37a90871e/src/java.base/share/classes/java/lang/String.java#L1879-L1880; I think it's because checking byte per byte equality (from a semantic perspective), still hold. I didn't checked what are the other usage for the UTF16 version, but @cl4es could clarify it . > The equals method will do the tv and ov length comparisons themselves, could > we remove that check here or is the method call overhead too large? I didn't remove it because region equality won't return (always) false, as equals would do, in such case, but will compare the (If existing) minimal common available length (till the specified len) between the two. > Do you happen to have results for make test > TEST="micro:java.lang.StringComparisons"? Not yet, I have performed a microbenchmark myself to verify that with small strings (below 16 bytes), equals was much better (both by enabling and disabling the mismatch intrinsics), but I will do it, thanks! I'm missing as well to verify the bytecode size of the method after my changes, to make sure I didn't make it non-inlineable. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16933#discussion_r1413009110 PR Review Comment: https://git.openjdk.org/jdk/pull/16933#discussion_r1413008371
