Performance improvements for _gej_add_ge #267

pull peterdettman wants to merge 5 commits into bitcoin-core:master from peterdettman:perf-gej-ge changing 1 files +17 −20

peterdettman commented at 7:58 AM on July 4, 2015: contributor

A few local refactorings in secp256k1_gej_add_ge() that give a small performance improvement; I measure about 1.4% here (bench_sign).

Perf. improvement in _gej_add_ge

- Avoid one weak normalization
- Change one full normalization to weak
- Avoid unnecessary fe assignment
- Update magnitude annotations

55e7fc32cb

Refactor to remove a local var b28d02a5d5
Refactor to save a _fe_negate 7d054cd030

in src/group_impl.h:None in 7d054cd030 outdated

 529 |      t = u1; secp256k1_fe_add(&t, &u2);                  /* t = T = U1+U2 (2) */
 530 |      m = s1; secp256k1_fe_add(&m, &s2);                  /* m = M = S1+S2 (2) */
 531 |      secp256k1_fe_sqr(&rr, &t);                          /* rr = T^2 (1) */
 532 | -    secp256k1_fe_mul(&tt, &u1, &u2); secp256k1_fe_negate(&tt, &tt, 1); /* tt = -U1*U2 (2) */
 533 | -    secp256k1_fe_add(&rr, &tt);                                        /* rr = R = T^2-U1*U2 (3) */
 534 | +    secp256k1_fe_negate(&m_alt, &u2, 1);                /* m = -X2*Z1^2 */

apoelstra commented at 3:17 PM on July 6, 2015:

The comment should say Malt rather than m. (This was also a problem with the line in its old position, oops.)

in src/group_impl.h:None in 7d054cd030 outdated

 541 | @@ -542,7 +542,6 @@ static void secp256k1_gej_add_ge(secp256k1_gej_t *r, const secp256k1_gej_t *a, c
 542 |       * so we set R/M equal to this. */
 543 |      secp256k1_fe_negate(&rr_alt, &s2, 1);   /* rr = -Y2*Z1^3  */
 544 |      secp256k1_fe_add(&rr_alt, &s1);         /* rr = Y1*Z2^3 - Y2*Z1^3 */
 545 | -    secp256k1_fe_negate(&m_alt, &u2, 1);    /* m = -X2*Z1^2 */
 546 |      secp256k1_fe_add(&m_alt, &u1);          /* m = X1*Z2^2 - X2*Z1^2 */

apoelstra commented at 3:17 PM on July 6, 2015:

This m should also be Malt.

apoelstra commented at 3:34 PM on July 6, 2015: contributor

Tested ACK, aside from the comment fixes I mentioned. Confirmed the 1.4% perf improvement (looks like 1.6 to me).
Update code comments a5d796e0b1
Save 1 _fe_negate since s1 == -s2 5a43124c69
sipa merged this on Jul 8, 2015
sipa closed this on Jul 8, 2015
sipa referenced this in commit 06ff7fe5b0 on Jul 8, 2015

Contributors

peterdettman

apoelstra