Remove the assert in Integer.valueOf()

[Ulf Zibis]

Hi Rémi,

Am 28.04.2012 00:42, schrieb Rémi Forax:
>> While you are there:
>> IntegerCache.cache/high/low are static final, so should be named _upper case_.
>
> Not done because the system property is also spelled in lower case.
Hm, but does that justify violating very common code conventions?
IMO it's inconvenient while reading the code.

>
>>
>> Another optimization:
>>     public static Integer valueOf(int i) {
>>         if (i >= 0) {
>>             if (i <= IntegerCache.HIGH)
>>                 return IntegerCache.POS[i];
>>         } else {
>>             if (i >= IntegerCache.LOW)
>>                 return IntegerCache.NEG[~i];
>>         }
>>         return new Integer(i);
>>     }
>>
>
> Can you explain a little bit more why it's more efficient ?
1. Compare against 0 is little faster than against a constant which must be loaded as operand.
2. As positive arguments should be more frequent, my version prevents from calculating the index 
every time:
     IntegerCache.cache[i + (-IntegerCache.low)]
3. Client VM would profit from less bytecode instructions to go through, as there is less inlining.

> Given that low and high are constant for the JIT, I'm not sure the current code
> do bounds checking at all.
I do not understand this. 'i' is not constant, so should be checked.

==========================================

We must be careful with Integer.MAX_VALUE as maximum array size.
See: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7153247
and: java.lang.AbstractCollection.MAX_ARRAY_SIZE

Additionally, I think, *the assert is superfluous at all*, as 'high' can never be smaller than 127.
You can see this, if you compact the code:

         static {
             // high value may be configured by property
             String cacheHigh =
                 sun.misc.VM.getSavedProperty("java.lang.Integer.IntegerCache.high");
             /********** no need for variable i : **********/
             int h = 127;
             if (cacheHigh != null) {
                 h = Math.max(parseInt(cacheHigh), 127);
                 // Maximum array size is Integer.MAX_VALUE
                 h = Math.min(h, Integer.MAX_VALUE - (-low));
                 /********** little shorter : **********/
                 // Maximum array size is Integer.MAX_VALUE
                 h = Math.min(Integer.MAX_VALUE - (-low),
                         Math.max(parseInt(cacheHigh), 127));
             }
             high = h;
             /********** more simple : **********/
             int h = 0;
             if (cacheHigh != null) {
                 // Maximum array size is Integer.MAX_VALUE
                 h = Math.min(Integer.MAX_VALUE - (-low),
                         parseInt(cacheHigh));
             }
             high = Math.max(h, 127);
             /********** no need for variable h : **********/
             if (cacheHigh != null)
                 // Maximum array size is Integer.MAX_VALUE
                 high = Math.min(Integer.MAX_VALUE - (-low),
                         Math.max(parseInt(cacheHigh), 127));
             else
                 high = 127;
             /********** shorter : **********/
             high = (cacheHigh == null) ? 127 :
                     // Maximum array size is Integer.MAX_VALUE
                     Math.min(Integer.MAX_VALUE - (-low),
                             Math.max(parseInt(cacheHigh), 127));
             /********** most short : **********/
             // Maximum array size is Integer.MAX_VALUE
             high = Math.max(127, Math.min((cacheHigh == null) ? 127 :
                     parseInt(cacheHigh), Integer.MAX_VALUE - (-low)));

             assert IntegerCache.high >= 127;

             // range [-128, 127] must be interned (JLS7 5.1.7)
             cache = new Integer[(high - low) + 1];
             for(int j = low, k = 0; k < cache.length; k++, j++)
                 cache[k] = new Integer(j);
         }


-Ulf