RFR: 8079136: Accessing a nested sublist leads to StackOverflowError

[Martin Buchholz]

On Thu, May 7, 2015 at 9:18 AM, Ivan Gerasimov <ivan.gerasimov at oracle.com>
wrote:

>
>  ---
>>
>> I would make modCount an argument to updateSizeAndModCount.
>>
>>  I did that initially, but later realized that this argument should never
> be different from root.modCount.
>
>
Sure, but you'd keep it in a local variable in the loop instead of a field
read.  No big deal either way.


>  ---
>>
>> Separate out hot and cold code in subListRangeCheck (although
>> pre-existing code had the same problem)
>>
>> +    static void subListRangeCheck(int fromIndex, int toIndex, int size) {
>> +        if (fromIndex < 0)
>> +            throw new IndexOutOfBoundsException("fromIndex = " +
>> fromIndex);
>> +        if (toIndex > size)
>> +            throw new IndexOutOfBoundsException("toIndex = " + toIndex);
>> +        if (fromIndex > toIndex)
>> +            throw new IllegalArgumentException("fromIndex(" + fromIndex +
>> +                                               ") > toIndex(" + toIndex
>> + ")");
>> +    }
>> +
>>
>> if ((fromIndex < 0) | (toIndex > size) | (fromIndex > toIndex))
>> slowpath();
>>
>>  Recently, I investigated this possible way of optimization, and I was
> surprised to discover that javac generates more compact code for logical
> operators, comparing to bitwise operators for booleans.
>
> I've just tried it again to refresh my memory and here are results for a
> simple function:
>
> This >>> void f(int index) { if (index < 0 || index >= size) throw new
> Error(); } <<< compiles into 20 bytecode commads.
> This >>> void f(int index) { if (index < 0 | index >= size) throw new
> Error(); } <<< produces 34 commads.
>
> This is because the bitwise or operator really operates on integers.
> So, javac first conditionally initializes an auxiliary integer variables
> to either 0 or 1, then ORs them, and then checks the result.
> As the result, we have 3 branches in the code above.
>
> Another approach would be to do something like this:
>     void f(int index) {
>         int x = index | (size - 1 - index);
>         if (x < 0)
>             throw new Error();
>     }
> The code length (23) is still a bit greater, comparing to the first
> version, but on the other hand it's got only one branch.
> It's still not clear though, if this going to be more efficient or not.
>
> I recommend to leave this code as is for now, and investigate the ways to
> optimize it in a different CR.


You make a good point about code size of logical operations on booleans.  I
don't know which will be more efficient in practice - it depends on the
hotspot implementation.  I agree with your analysis.