On Wed, 9 Mar 2022 23:44:22 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> I'm requesting comments and, hopefully, some help with this patch to replace >> `StringCoding.hasNegatives` with `countPositives`. The new method does a >> very similar pass, but alters the intrinsic to return the number of leading >> bytes in the `byte[]` range which only has positive bytes. This allows for >> dealing much more efficiently with those `byte[]`s that has a ASCII prefix, >> with no measurable cost on ASCII-only or latin1/UTF16-mostly input. >> >> Microbenchmark results: >> https://jmh.morethan.io/?gists=428b487e92e3e47ccb7f169501600a88,3c585de7435506d3a3bdb32160fe8904 >> >> - Only implemented on x86 for now, but I want to verify that implementations >> of `countPositives` can be implemented with similar efficiency on all >> platforms that today implement a `hasNegatives` intrinsic (aarch64, ppc etc) >> before moving ahead. This pretty much means holding up this until it's >> implemented on all platforms, which can either contributed to this PR or as >> dependent follow-ups. >> >> - An alternative to holding up until all platforms are on board is to allow >> the implementation of `StringCoding.hasNegatives` and `countPositives` to be >> implemented so that the non-intrinsified method calls into the intrinsified. >> This requires structuring the implementations differently based on which >> intrinsic - if any - is actually implemented. One way to do this could be to >> mimic how `java.nio` handles unaligned accesses and expose which intrinsic >> is available via `Unsafe` into a `static final` field. >> >> - There are a few minor regressions (~5%) in the x86 implementation on >> `encode-/decodeLatin1Short`. Those regressions disappear when mixing inputs, >> for example `encode-/decodeShortMixed` even see a minor improvement, which >> makes me consider those corner case regressions with little real world >> implications (if you have latin1 Strings, you're likely to also have >> ASCII-only strings in your mix). > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Revert encodeUTF8 for this PR due issues with fragile optimization Reverting changes to `String.encodeUTF8` brought all `encode`-micros down to effectively no change: https://jmh.morethan.io/?gist=b957cb9457c31141ac71d47f2e10486a (which proves implementing `hasNegatives` using `countPositives != len` has no measurable cost) I consider this the final version for this PR (assuming tests pass). I need someone to review the aarch64 changes in particular, and perhaps someone from the core library team should sign off on the String changes (less of those now). ------------- PR: https://git.openjdk.java.net/jdk/pull/7231