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

Reply via email to