Hi, Alexei!
Thanks for the review! Here is some light on your questions:
On Thu, May 22, 2008 at 11:27 AM, Alexei Fedotov
wrote:
> 1. I have noticed that you have replaced shiftLeft(1) with
> shiftLeftOneBit() in the following checks which appear . Is it
> performance critical part or just cleaning?
This is performance-wise change. "sign == 0" guarantees that shifting
is not needed.
> // Checking if: remainder * 2 >= scaledDivisor
> remainder.abs().shiftLeftOneBit().compareTo(scaledDivisor.abs())
>
> If this is critical, then can one imagine a way of checking that a
> number is twice bigger than another number without copying and
> shifting the number? For cases when one number has more binary digits
> than another this should be easy.
IMO, this way is also fast, though we can think up about something
with peak efficiency. I see we will need structural changes to
accommodate another design which would allow us to use other checking
schemes.
> 2. What is the meaning of 2 in the new function name unsignedMultAdd2?
> Why not unsignedMultAdd4?
That mean "unsigned multiplication and addition of 2 arguments", hence
the "2" in signature. We can change it to another if this is
confusing.
> 2a. What is the purpose of the following fix? How a function call
> (even for a magic function), could be quicker than multiplication
> inlined by JIT? Does it mean our JIT poorly inlines multiplication?
> - long res = (aDigits[0] & 0xFFFFFFFFL) * (factor);
> + long res = unsignedMultAdd2(aDigits[0], factor, 0, 0);
Well, the original purpose of this was code cleanup (see how original
version look like, bloated with 0xF...F constants), so we came up with
idea of extracting unsigned operations of (a*b), (a*b + c), (a*b + c +
d) as general method. Then you can imagine two situations:
1. JIT compiler inlines the method and eliminated unneeded additions.
2. JIT compiler generates the magic for this method and substituting
it with optimal native code (HARMONY-5826).
*sigh* I guess I need to write the javadoc for this method.
> 3. You added a check against shifting zero values. Does adding this
> check on a common path add to performance? If zeroes come to shift
> function often enough to give a benefit, which caller spams so much
> zeroes? Does it worth to patch the caller instead?
Nope, the benefit of 0-path there is much more than penalty on non-0-path.
> 4. I have an abstract concern with no idea how to address it. An
> integer array in memory is very similar to a long array (if endings
> are correct). Is there any way to use 64-bit operations for speed up?
We thought about such the thing. This would require severe changes to
fit into existing code. More details would be later :)
Thanks,
Aleksey.