On 09/26/2018 06:39 PM, raffaello.giulietti@gmail.com wrote:
The submitted code contains both the changes to the current implementation and extensive jtreg tests.
While I've struggled to keep the code within the 80 chars/line limit, mercurial still generates longer lines. Thus, to avoid possible problems with the email systems, the code is submitted both inline and as an attachment. Hope at least one copy makes its way without errors.
Overall, the commenting is much too light. There are many places where I think I know what you're doing but you don't explain it. Here, for example:
+ + // pow5 = pow51*2^63 + pow50 + long pow51 = ceilPow5dHigh(-k); + long pow50 = ceilPow5dLow(-k); + + // p = p2*2^126 + p1*2^63 + p0 and p = pow5 * cb + long x0 = pow50 * cb; + long x1 = multiplyHigh(pow50, cb); + long y0 = pow51 * cb; + long y1 = multiplyHigh(pow51, cb); + long z = (x1 << 1 | x0 >>> 63) + (y0 & MASK_63); + long p0 = x0 & MASK_63; + long p1 = z & MASK_63; + long p2 = (y1 << 1 | y0 >>> 63) + (z >>> 63); + long vn = p2 << 1 + ord2alpha | p1 >>> 62 - ord2alpha; + if ((p1 & mask) != 0 || p0 >= threshold) { + vn |= 1; + }
... etc. I think I can figure out what you're doing, but you could explain it. If you write the comments now while the code is still fresh in your mind it'll be easy.
+ private static final long[] ceilPow5d = { + /* -292 */ 0x7FBB_D8FE_5F5E_6E27L, 0x497A_3A27_04EE_C3DFL, + /* -291 */ 0x4FD5_679E_FB9B_04D8L, 0x5DEC_6458_6315_3A6CL, + /* -290 */ 0x63CA_C186_BA81_C60EL, 0x7567_7D6E_7BDA_8906L, + /* -289 */ 0x7CBD_71E8_6922_3792L, 0x52C1_5CCA_1AD1_2B48L,
What exactly is this table anyway? How was it generated? Please say. There are many more places in the code. What you've done is nice, but it could be exemplary. -- Andrew Haley Java Platform Lead Engineer Red Hat UK Ltd. <https://www.redhat.com> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671