RFR: 8334755: Asymptotically faster implementation of square root algorithm [v30]
Raffaello Giulietti
rgiulietti at openjdk.org
Thu Jul 18 15:09:40 UTC 2024
On Thu, 18 Jul 2024 14:55:30 GMT, fabioromano1 <duke at openjdk.org> wrote:
>> src/java.base/share/classes/java/math/MutableBigInteger.java line 2135:
>>
>>> 2133: * @param limit the offset which is the end of valid words in the input value
>>> 2134: * @param blockLen the length of the block in units of 32 bits
>>> 2135: */
>>
>> At first, I couldn't understand that `blockIndex` goes in the opposite direction as the array indices. Also, "starting" is very confusing, because no `int` at `blockIndex*blockLen` gets copied AFAIU.
>>
>> Two improvements:
>> * Add a `@return` tag. Alternatively, use comments other than JavaDoc `/**`.
>> * Make it explicit that the direction of `blockIndex` is the opposite of the array indices, and reconsider the wording like "starting", etc.
>
> For the sake of information: the documentation of this method is actually based on that of `MBI.getBlock(int, int, int)`, which has the same problems.
I agree, and one could keep improving MBI's doc a lot, in particular implicit assumptions.
But let's concentrate on this PR and make this code readable and well documented.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19710#discussion_r1683018032
More information about the core-libs-dev
mailing list