Re: RFR 8017540: Improve multi-threaded contention behavior of BigInteger.toString() radix conversion cache

[Peter Levart]

On 06/25/2013 01:53 AM, Brian Burkhalter wrote:
Branching off from this thread
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/018244.html

I filed this issue (should be public tomorrow)

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8017540

for the getRadixConversionCache() enhancement. The patch is here

http://cr.openjdk.java.net/~bpb/8017540/

I reviewed and tested the code for correctness and see no issues, so to speak. 
This patch cannot be integrated prior to that for 4641897.

Thanks,

Brian

Hi,

I think this has similar problem (data-race followed by read from 
half-constructed array) as the prototype that Aleksey constructed here 
(sorry, I have not seen the forked thread then yet):

http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/018368.html


    private static BigInteger getRadixConversionCache(int radix, int exponent) {
        BigInteger retVal = null;
        BigInteger[][] pc = powerCache; // volatile read
        BigInteger[] cacheLine = pc[radix];
        int oldSize = cacheLine.length;
        if (exponent >= oldSize) {
            cacheLine = Arrays.copyOf(cacheLine, exponent + 1);
            for (int i = oldSize; i <= exponent; i++) {
                retVal = cacheLine[i - 1].square();
                cacheLine[i] = retVal;
            }
            pc[radix] = cacheLine;
            powerCache = pc; // publish by volatile write
        } else {
            retVal = cacheLine[exponent];
        }

        return retVal;
     }



Thread A:
    pc = powerCache;

Thread B:
    pc = powerCache;
    cacheLine = pc[radix];
    oldSize = cacheLine.length;
    // enter if, since exponent >= oldSize
    cacheLine = copyOf(cacheLine, exponent+1);
    // ...for loop filling cacheLine...
    pc[radix] = cacheLine;

Thread A:
    cacheLine = pc[radix]; // this can read new cache line constructed 
by Thread B (data-race)
    oldSize = cacheLine.length;
    // skip if, since exponent < oldSize, enter else
    retVal = cacheLine[exponent]; // this can read null, since 
reference to cacheLine was obtained via data-race
    return retval;


What do you think of the code proposed here:

http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/018368.html


Regards, Peter