On Tue, 10 Aug 2021 21:06:00 GMT, Сергей Цыпанов 
<github.com+10835776+stsypa...@openjdk.org> wrote:

>> The code in `Integer.decode()` and `Long.decode()` might allocate two 
>> instances of Integer/Long for the negative values less than -127:
>> 
>> Integer result;
>> 
>> result = Integer.valueOf(nm.substring(index), radix);
>> result = negative ? Integer.valueOf(-result.intValue()) : result;
>> 
>> To avoid this we can declare 'result' as `int` and use `Integer.parseInt()` 
>> method. Same applicable for `Long` and some other classes.
>
> Сергей Цыпанов has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains five additional 
> commits since the last revision:
> 
>  - 8267844: Add benchmarks for Integer/Long.decode()
>  - 8267844: Rid useless substring when calling Integer/Long.parse*()
>  - Merge branch 'master' into 8267844
>  - Merge branch 'master' into 8267844
>  - 8267844: Replace Integer/Long.valueOf() with Integer/Long.parse*() where 
> applicable

Hi,
i check Long.java line 1301...1311 and i am not sure if this code is really 
good.
1) If negative  is false the whole part ends with two times the same substring 
and this implies the same error.
2) If negative is true and we get an error than we can throw the error without 
an second parse step or we can use the substring from the first round.
3) Also as mentioned above the parseLong(text,radix) should be changed to 
parseLong(seq, beginIndex, endIndex, radix)
  this would avoid at least in the positive case the substring at all.
4) The same points are with Integer as well. 
        try {
            result = parseLong(nm.substring(index), radix);
            result = negative ? -result : result;
        } catch (NumberFormatException e) {
            // If number is Long.MIN_VALUE, we'll end up here. The next line
            // handles this case, and causes any genuine format error to be
            // rethrown.
            String constant = negative ? ("-" + nm.substring(index))
                                       : nm.substring(index);
            result = parseLong(constant, radix);
        }

-------------

PR: https://git.openjdk.java.net/jdk/pull/5068

Reply via email to