On Thu, 19 Oct 2023 07:04:50 GMT, Chen Liang <li...@openjdk.org> wrote:
> Please review a patch that renames `JavaLangAccess::newStringUTF8NoRepl` and > `getBytesUTF8NoRepl` to `newStringUTF8FailFast` and `getBytesUTF8FailFast` to > accurately describe these APIs. This also renames other associated methods. > > NoRepl means "no replication", which is not what these APIs are for. They > instead expose the fail-fast construction fast-path in String, allowing > `ZipCoder` to quickly throw encoding exceptions wrapped in > `IllegalArgumentException` without going through decoders and encoders. > > In addition, there is already `newStringNoRepl` and `getBytesNoRepl` in > `JavaLangAccess` that actually avoids copying arrays for trusted usages, > further increasing the confusion. Given these internal APIs are frequently > used in recent contributions around improving string performance, I believe a > rename of these APIs is necessary. > > Pinging @cl4es for a review since you use this particular API the most often. On second thought, an alternative solution is to consolidate the String access internal APIs into: String newStringFailFast(byte[] bytes, Charset charset, boolean trusted); byte[] getBytesFailFast(String string, Charset charset, boolean trusted); These APIs will immediately throw encoding errors as an IAE with a CCE cause instead of silently performing a character substitution. There are 2 flags we want to consider: `trusted` and `failFast`. | | `trusted == true` | `trusted == false`| |-|--------|--------| | __`failFast == true`__| Usual trusted scenario | Usual fail-fast scenario | | __`failFast == false`__| Meaningless, if failure happens, trust is already broken | Can be done with public APIs | In conclusion, we can simplify the internal API to assume `failFast` holds true and only request a `trusted` flag. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16260#issuecomment-1770215673
