JDK 9 RFR of JDK-8035279: Clean up internal deprecations in BigInteger

[Paul Sandoz]

On Feb 27, 2014, at 2:50 PM, Peter Levart <peter.levart at gmail.com> wrote:

> On 02/27/2014 09:22 AM, Paul Sandoz wrote:
>> On Feb 27, 2014, at 12:51 AM, Brian Burkhalter <brian.burkhalter at oracle.com> wrote:
>> 
>>> On Feb 26, 2014, at 1:53 PM, Paul Sandoz wrote:
>>> 
>>>>> Thanks for the suggestion, Paul. Assuming it is correct, in what way would this be a better approach? (I apologize for being obtuse.)
>>>>> 
>>>> IMHO it is a simpler solution. It's a cache line that requires updating not the cache line*s*, as the current approach can potentially knock out cache lines for other radixes.
> 
> Potentially yes, but not very likely, since the whole powerCache reference is re-read again *after* the computation of cacheLine and then replaced with a modified clone immediately after that. It should be noted what Alan Eliasen already pointed-out in a discussion back in June 2013:
> 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/018375.html
> 

Yes, i was suggesting a CAS could be used to avoid replacing with a smaller cache line:

        while (!UnsafeHolder.casCacheLine(powerCache, radix, cacheLine, newCacheLine)) {
            cacheLine = powerCache[radix];
            if (exponent < cacheLine.length) {
                break;
            }
        }

but it can still result in two or more threads duplicating work as you say below, dunno how critical that is. I suppose if i was super paranoid about this i write some initialization code to warm the cache up :-) also being super paranoid perhaps there should be a limit of exponents that are cached?


> ...so I think what could be improved in current implementation is the potential and more likely useless computation of the same values for the same radix by multiple concurrent threads.



> Back then I presented an alternative solution which solves that:
> 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/018457.html
> 
> ...and as can be seen, does not use volatile read/write to publish BigIntegers - that's why it is important that internal implementation of such immutable classes like BigInteger is tolerant to unsafe publication...
> 

Interesting, nice use of a linked list. Not sure that is safe publication of cacheLine into powerCache, the last element in a line could be observed as null, plus although it is unlikely, the compiler could shuffle up the array element store?

Paul.