On Mon, 4 Dec 2023 10:51:43 GMT, Claes Redestad <[email protected]> wrote:
>>> 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. > > Yes, since the `coder` is checked first we know the bytes compared are either > both latin1 or both UTF16 so there's no need for a UTF-16-aware char-by-char > comparison here. In fact the `StringUTF16.equals` method looks completely > unused. I can send a separate PR to remove it in case, unless we have any plans to use it elsewhere ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16933#discussion_r1413765820
