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