Skip to content

ci: drop -f from test compile flags to speed up CI - #16

Closed
chaitanyaprem wants to merge 2 commits into
chore/bump-libp2p-v2.0.0from
chore/drop-f-from-test-compile
Closed

ci: drop -f from test compile flags to speed up CI#16
chaitanyaprem wants to merge 2 commits into
chore/bump-libp2p-v2.0.0from
chore/drop-f-from-test-compile

Conversation

@chaitanyaprem

Copy link
Copy Markdown
Collaborator

Drops the -f (force recompile) flag from the shared cfg used by every runTest and buildExample invocation in libp2p_mix.nimble.

Why

Each test in nimble test / nimble testComponent is its own Nim binary, but they all share a single nimcache/ directory in the working tree. With -f present, every test recompiles its entire transitive dep tree (libp2p, chronos, stew, nimcrypto, …) from scratch — the dominant cost of CI.

Without -f, the first test pays the full compile cost; subsequent tests reuse compiled .o files from nimcache. Expected per-job reduction: ~50-70% of the test-phase wall-clock.

Baseline (from PR #14's CI run)

Stacked on top of #12 (chore/bump-libp2p-v2.0.0). Before this change, the test-phase per-job times on a fresh runner were:

Job Unit tests Component tests Total
linux-amd64 (Nim 2.2.4) 13m54s (similar) ~26-28m
linux-amd64 (Nim 2.2.10) 14m10s (similar) ~26-28m
linux-i386 (Nim 2.2.4) 14m20s (similar) ~26-28m
linux-i386 (Nim 2.2.10) 14m22s (similar) ~26-28m
macos-arm64 (Nim 2.2.4) 8m56s (similar) ~22m
macos-arm64 (Nim 2.2.10) 9m19s 12m05s 26m57s

CI for this PR should show the same number of tests run with substantially lower wall-clock — that's the validation.

Why this is safe

  • All test invocations share identical compile flags. Both nimble test and nimble testComponent call runTest(name) with no per-test moreoptions. Every test compiles with the same cfg, so nimcache hits will always be flag-correct.
  • The retained flags are intentional: --styleCheck:usages, --styleCheck:error, --opt:speed, --threads:on, -d:libp2p_mix_experimental_exit_is_dest. Only the recompile-everything bit is gone.
  • If a future test needs different flags, pass them via moreoptions and pair with -f for that specific invocation rather than re-enabling it globally.

Diff

One file, ~9 lines added (most of which is an explanatory comment):

 let cfg =
   " --styleCheck:usages --styleCheck:error" & (if verbose: "" else: " --verbosity:0") &
-  " --skipUserCfg -f --threads:on --opt:speed" &
+  " --skipUserCfg --threads:on --opt:speed" &
   " -d:libp2p_mix_experimental_exit_is_dest"
+  # `-f` (force recompile) was previously here. Dropped so test binaries
+  # share the nimcache: each subsequent test in `nimble test` /
+  # `nimble testComponent` reuses compiled libp2p / chronos / stew object
+  # files from the first test, cutting per-test compile time substantially.
+  # Safe because every test invokes `runTest(name)` with no per-test
+  # compile flag overrides — all tests build with identical `cfg`, so
+  # cache hits are flag-correct.

Rollback

If this surfaces flaky behavior in CI (test cross-contamination via stale nimcache), the rollback is a one-character revert — re-add -f in cfg. No downstream impact since libp2p_mix.nimble's test config is purely internal to this repo.

Each test in `nimble test` and `nimble testComponent` is its own Nim
binary, sharing a single nimcache. The `-f` (force recompile) flag was
forcing every test to recompile its entire transitive dep tree (libp2p,
chronos, stew, nimcrypto, ...) from scratch — the dominant cost of CI.

Without `-f`, the first test pays the full compile cost; subsequent
tests reuse compiled `.o` files from nimcache. Expected per-job
reduction: ~50-70% of the test-phase wall-clock.

Safe to drop because:
- All test invocations go through `runTest(name)` with the default
  empty `moreoptions`, so every test compiles with identical `cfg` —
  nimcache hits will always be flag-correct.
- `--styleCheck`, `--opt:speed`, `-d:libp2p_mix_experimental_exit_is_dest`
  remain in `cfg`; only the recompile-everything bit is gone.

If a future test needs different compile flags, pass them via
`moreoptions` and pair with `-f` for that specific invocation rather
than re-enabling it globally.
nph reflows the last two `&`-joined `cfg` strings onto a single line now
that removing `-f` brought the line below the wrap threshold. No
functional change.
@chaitanyaprem

Copy link
Copy Markdown
Collaborator Author

Closing — the -f removal turned out to be a no-op on the test phase.

Why it didn't work: each nim c tests/test_X.nim invocation defaults to its own nimcache/test_X_d/ subdirectory (keyed on the entry-point filename). The 14 unit + 6 component tests in a single CI job never shared compilation in the first place, so removing -f (the force-recompile flag) couldn't unlock anything.

CI on this PR vs the prior baseline (PR #14):

  • linux-amd64 (2.2.10): 36m18s vs ~26-28m in-flight (no meaningful change)
  • linux-i386 (2.2.10): 37m35s — same range
  • macos-arm64 (2.2.10): 25m15s vs 26m57s (~6%, within runner variance)

Tracked as # for proper investigation (likely fix: explicit --nimcache:<shared-path> so all tests share compiled object files).

@chaitanyaprem

Copy link
Copy Markdown
Collaborator Author

Tracked in #17.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant