ci: Various improvements #1047

pull real-or-random wants to merge 6 commits into bitcoin-core:master from real-or-random:202112-ci-improvements changing 4 files +61 −74
  1. real-or-random commented at 12:10 pm on December 21, 2021: contributor

    This is a replacement for #1046. It contains only the changes that (I think) are unlikely to be debated and don’t interfere with #995. I’d like to get this in quickly to get a green badge again. (Using 2 CPUs instead of one for valgrind should do the trick.)

    I’ll open another PR on top of this to optimize the matrix. This will deserve more discussion and we should get #995 merged first.

  2. precompute_ecmult: Always compute all tables up to default WINDOW_G
    Also simplify #ifdefs in generated file.
    10461d8bd3
  3. ci: Remove STATICPRECOMPUTATION
    This has been overlooked in #988.
    26a022a3a0
  4. real-or-random cross-referenced this on Dec 21, 2021 from issue ci: Optimize build matrix by real-or-random
  5. real-or-random commented at 12:35 pm on December 21, 2021: contributor
    For more background on the “greedy” flag, see https://github.com/bitcoin/bitcoin/pull/23797.
  6. in .cirrus.yml:7 in 965f9480c4 outdated
    3@@ -4,10 +4,10 @@ env:
    4   # Specific warnings can be disabled with -Wno-error=foo.
    5   # -pedantic-errors is not equivalent to -Werror=pedantic and thus not implied by -Werror according to the GCC manual.
    6   WERROR_CFLAGS: -Werror -pedantic-errors
    7-  MAKEFLAGS: -j2
    8+  MAKEFLAGS: -j4
    


    elichai commented at 12:37 pm on December 22, 2021:
    Does the CI server actually have 4 CPUs?

    real-or-random commented at 12:45 pm on December 22, 2021:
    We set cpu: 1 or cpu: 2 depending on the task. But the greedy flag will occasionally give us more CPUs when available. Increasing this to -j4 makes sure we actually can make use of this.
  7. in .cirrus.yml:91 in 965f9480c4 outdated
    87@@ -84,10 +88,7 @@ task:
    88 
    89 task:
    90   name: "i686: Linux (Debian stable)"
    91-  container:
    92-    dockerfile: ci/linux-debian.Dockerfile
    93-    cpu: 1
    94-    memory: 1G
    95+  << : *LINUX_CONTAINER
    


    elichai commented at 12:38 pm on December 22, 2021:
    Is this a yaml trick?

    real-or-random commented at 12:43 pm on December 22, 2021:
    Yes, we use this in other places, too.

    elichai commented at 12:47 pm on December 22, 2021:
    Found it: https://yaml.org/type/merge.html This is interesting, I did not know yaml has this sort of things

    real-or-random commented at 12:53 pm on December 22, 2021:
    Yeah, core uses it all over the place, so we’re in good company.
  8. in .cirrus.yml:140 in 965f9480c4 outdated
    134@@ -134,8 +135,9 @@ task:
    135   ##   - rm /tmp/.com.apple.dt.CommandLineTools.installondemand.in-progress
    136   ##
    137   brew_valgrind_pre_script:
    138+    - brew update
    139     - brew config
    140-    - brew tap --shallow LouisBrunner/valgrind
    141+    - brew tap LouisBrunner/valgrind
    


    elichai commented at 12:39 pm on December 22, 2021:
    Why not shallow?

    real-or-random commented at 12:43 pm on December 22, 2021:
    Shallow doesn’t exist anymore in recent brew versions, see the commit message.
  9. elichai commented at 12:42 pm on December 22, 2021: contributor
    Manually checked that the ECMULT_TABLE_SIZE changes are correct according to (1 << (w-2))
  10. elichai commented at 12:56 pm on December 22, 2021: contributor
    utACK 965f9480c417254318a3093936ee9748391b49b1 The C file changes are simple and correct, and it’s great to test different window sizes
  11. in .cirrus.yml:77 in 965f9480c4 outdated
    75     - env: {BUILD: distcheck, WITH_VALGRIND: no, CTIMETEST: no, BENCH: no}
    76     - env: {CPPFLAGS: -DDETERMINISTIC}
    77     - env: {CFLAGS: -O0, CTIMETEST: no}
    78-    - env: { ECMULTGENPRECISION: 2 }
    79-    - env: { ECMULTGENPRECISION: 8 }
    80+    - env: { ECMULTGENPRECISION: 2, ECMULTWINDOW: 4 }
    


    jonasnick commented at 1:47 pm on December 22, 2021:
    Better to check the edges? so ECMULTWINDOW: 2?

    real-or-random commented at 1:53 pm on December 22, 2021:
    Indeed, let me just change to checking 2 (as edge case) and 4, which is a more reasonable value because it still needs almost no memory and may be a candidate for a “lowmem” setting here we may introduce in the future. (Rust bindings do the same, see https://github.com/rust-bitcoin/rust-secp256k1/blob/master/secp256k1-sys/build.rs#L45)

    real-or-random commented at 1:57 pm on December 22, 2021:
    fixed with force-push
  12. ci: Test different ecmult window sizes 22382f0ea0
  13. ci: Update brew on macOS
    The preinstalled brew is very old and tries to download prebuilt bottles
    from a server which is no longer available. Because that will fail, brew
    falls back to building our dependencies (e.g., autotools) from source,
    which takes very long.
    
    This commit makes sure that brew is updated before we start the build.
    
    We also need to remove the `--shallow` argument from `brew tap`. It
    doesn't exist in recent brew versions.
    d07e30176e
  14. ci: Use Cirrus "greedy" flag to use idle CPU time when available e70acab601
  15. ci: Run valgrind/memcheck tasks with 2 CPUs
    ... and increase the memory only for UBSan, ASan, LSan builds. Those are
    the ones who need more memory.
    b4ac1a1d5f
  16. real-or-random force-pushed on Dec 22, 2021
  17. jonasnick commented at 2:13 pm on December 22, 2021: contributor
    ACK b4ac1a1d5f4d51b9836ac310b78bc9d4256580c2
  18. elichai commented at 2:19 pm on December 22, 2021: contributor
    utACK b4ac1a1d5f4d51b9836ac310b78bc9d4256580c2
  19. jonasnick merged this on Dec 22, 2021
  20. jonasnick closed this on Dec 22, 2021

  21. sipa commented at 6:45 pm on December 22, 2021: contributor
    Posthumous ACK b4ac1a1d5f4d51b9836ac310b78bc9d4256580c2
  22. real-or-random cross-referenced this on Dec 22, 2021 from issue Add _fe_half and use in _gej_add_ge and _gej_double by peterdettman
  23. jonasnick cross-referenced this on Jan 2, 2022 from issue Sync Upstream by jonasnick
  24. real-or-random referenced this in commit 21e2d65b79 on Jan 5, 2022
  25. dhruv referenced this in commit 6f7e395acc on Jan 26, 2022
  26. hebasto referenced this in commit 065b6dbf9d on Jan 30, 2022
  27. dhruv referenced this in commit 139d4b881e on Feb 1, 2022
  28. fanquake referenced this in commit 8f8631d826 on Mar 17, 2022
  29. fanquake referenced this in commit 4bb1d7e62a on Mar 17, 2022
  30. fanquake referenced this in commit 465d05253a on Mar 30, 2022
  31. stratospher referenced this in commit 4d5afc9767 on Apr 3, 2022
  32. stratospher referenced this in commit 5b18493cfa on Apr 3, 2022
  33. fanquake referenced this in commit afb7a6fe06 on Apr 6, 2022
  34. gwillen referenced this in commit 35d6112a72 on May 25, 2022
  35. patricklodder referenced this in commit 21badcf9d2 on Jul 25, 2022
  36. patricklodder referenced this in commit 03002a9013 on Jul 28, 2022
  37. janus referenced this in commit 3a0652a777 on Aug 4, 2022
  38. in src/precompute_ecmult.c:61 in b4ac1a1d5f
    56@@ -57,6 +57,8 @@ static void print_two_tables(FILE *fp, int window_g) {
    57 }
    58 
    59 int main(void) {
    60+    /* Always compute all tables for window sizes up to 15. */
    61+    int window_g = (ECMULT_WINDOW_SIZE < 15) ? 15 : ECMULT_WINDOW_SIZE;
    


    binharebs commented at 7:33 pm on February 11, 2023:
    15_

    binharebs commented at 7:33 pm on February 11, 2023:
  39. in src/precompute_ecmult.c:81 in b4ac1a1d5f
    78@@ -77,15 +79,15 @@ int main(void) {
    79     fprintf(fp, "#include \"ecmult.h\"\n");
    80     fprintf(fp, "#include \"precomputed_ecmult.h\"\n");
    81     fprintf(fp, "#define S(a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p) SECP256K1_GE_STORAGE_CONST(0x##a##u,0x##b##u,0x##c##u,0x##d##u,0x##e##u,0x##f##u,0x##g##u,0x##h##u,0x##i##u,0x##j##u,0x##k##u,0x##l##u,0x##m##u,0x##n##u,0x##o##u,0x##p##u)\n");
    


    binharebs commented at 7:33 pm on February 11, 2023:

  40. in src/precompute_ecmult.c:80 in b4ac1a1d5f
    78@@ -77,15 +79,15 @@ int main(void) {
    79     fprintf(fp, "#include \"ecmult.h\"\n");
    80     fprintf(fp, "#include \"precomputed_ecmult.h\"\n");
    81     fprintf(fp, "#define S(a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p) SECP256K1_GE_STORAGE_CONST(0x##a##u,0x##b##u,0x##c##u,0x##d##u,0x##e##u,0x##f##u,0x##g##u,0x##h##u,0x##i##u,0x##j##u,0x##k##u,0x##l##u,0x##m##u,0x##n##u,0x##o##u,0x##p##u)\n");
    82-    fprintf(fp, "#if ECMULT_TABLE_SIZE(ECMULT_WINDOW_SIZE) > %ld\n", ECMULT_TABLE_SIZE(ECMULT_WINDOW_SIZE));
    


    binharebs commented at 7:34 pm on February 11, 2023:

  41. in src/precompute_ecmult.c:82 in b4ac1a1d5f
    78@@ -77,15 +79,15 @@ int main(void) {
    79     fprintf(fp, "#include \"ecmult.h\"\n");
    80     fprintf(fp, "#include \"precomputed_ecmult.h\"\n");
    81     fprintf(fp, "#define S(a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p) SECP256K1_GE_STORAGE_CONST(0x##a##u,0x##b##u,0x##c##u,0x##d##u,0x##e##u,0x##f##u,0x##g##u,0x##h##u,0x##i##u,0x##j##u,0x##k##u,0x##l##u,0x##m##u,0x##n##u,0x##o##u,0x##p##u)\n");
    82-    fprintf(fp, "#if ECMULT_TABLE_SIZE(ECMULT_WINDOW_SIZE) > %ld\n", ECMULT_TABLE_SIZE(ECMULT_WINDOW_SIZE));
    83+    fprintf(fp, "#if ECMULT_WINDOW_SIZE > %d\n", window_g);
    


    binharebs commented at 7:34 pm on February 11, 2023:
    __
  42. in src/precompute_ecmult.c:88 in b4ac1a1d5f
    86     fprintf(fp, "#ifdef EXHAUSTIVE_TEST_ORDER\n");
    87     fprintf(fp, "#    error Cannot compile precomputed_ecmult.c in exhaustive test mode\n");
    88     fprintf(fp, "#endif /* EXHAUSTIVE_TEST_ORDER */\n");
    89     fprintf(fp, "#define WINDOW_G ECMULT_WINDOW_SIZE\n");
    90 
    91-    print_two_tables(fp, ECMULT_WINDOW_SIZE);
    


    binharebs commented at 7:34 pm on February 11, 2023:

    binharebs commented at 7:34 pm on February 11, 2023:
    00

    binharebs commented at 7:35 pm on February 11, 2023:
    @
  43. binharebs commented at 7:36 pm on February 11, 2023: none
    995
  44. in src/precompute_ecmult.c:79 in b4ac1a1d5f
    78@@ -77,15 +79,15 @@ int main(void) {
    79     fprintf(fp, "#include \"ecmult.h\"\n");
    


    roconnor-blockstream commented at 3:12 pm on March 14, 2023:
    ecmult.h is no longer needed if you remove ECMULT_TABLE_SIZE.

    real-or-random commented at 5:45 pm on April 20, 2023:
    I think it’s correct to use ECMULT_TABLE_SIZE.

    roconnor-blockstream commented at 7:50 pm on April 21, 2023:
    Right sorry. For some reason I thought all the users of ECMULT_TABLE_SIZE were eliminated, but that is not true.
  45. str4d referenced this in commit 522190d5c3 on Apr 21, 2023
  46. vmta referenced this in commit e1120c94a1 on Jun 4, 2023
  47. vmta referenced this in commit 8f03457eed on Jul 1, 2023

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-22 11:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me