On Thu, 12 Oct 2023 00:39:27 GMT, David Schlosnagle <d...@openjdk.org> wrote:
>> He means to pull the `radix< Character,MIN_RADIX` and `radix > >> Character.MAX_RADIX` shared code in Integer and Long.parseInt (with radix >> version) to a helper method. > > More explicitly I was thinking something like below. > > I do wonder if some of the benchmark tests should cover the exceptional > cases. I have seen many systems where attempting to try and parse potentially > invalid inputs can hit expensive NumberFormatException fallbacks (though > `-XX:+OmitStackTraceInFastThrow` may eventually kick in). [Guava offers > `Integer > Ints.tryParse(String)`](https://guava.dev/releases/32.1.3-jre/api/docs/com/google/common/primitives/Ints.html#tryParse(java.lang.String)) > and similar to avoid the exception overhead at the cost of boxing for happy > path of valid `int` and `null` for invalid. > > > checkCanParseLength(s); > checkCanParseRadix(radix); > int len = s.length(); > > > private static void checkCanParseLength(String s) { > if (s == null) { > throw NumberFormatException.forNull(); > } > > if (s.isEmpty()) { // would be interesting to see if this is > equivalent to `s.length() == 0` for happy paths > throw NumberFormatException.forEmpty(); > } > } > > private static void checkCanParseRadix(int radix) { > if (radix < Character.MIN_RADIX) { > throw NumberFormatException.forMinRadix(radix); > } > > if (radix > Character.MAX_RADIX) { > throw NumberFormatException.forMaxRadix(radix); > } > } While only optimizing the fast path is a good idea, I think it is important to make sure there is no regression on the slow path - as @schlosna pointed out, it's a common way to check if a string can be converted to an int. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16112#discussion_r1356495786
