Defer instructionsToState evaluation in toTest (-62% wall time on a 12-worker run)#21
Open
CharlonTank wants to merge 1 commit intolamdera:mainfrom
Open
Defer instructionsToState evaluation in toTest (-62% wall time on a 12-worker run)#21CharlonTank wants to merge 1 commit intolamdera:mainfrom
CharlonTank wants to merge 1 commit intolamdera:mainfrom
Conversation
`toTest` was evaluating `instructionsToState instructions` eagerly while constructing the Test tree, just to read `state.testName`. With elm-test's parallel runner, every worker process forces the test tree on startup, so each worker re-runs the full simulation for every E2E test even though it only reports results for a small subset. This commit moves the `instructionsToState` call inside the `Test.test` lambda so the simulation is performed only when the test is actually executed by its assigned worker. The test name is now extracted via a new private `extractTestName` helper that walks the EndToEndTest chain without applying any NextStep/AndThen functions. Measured on a real-world Lamdera app with 14 E2E tests + 56 unit tests on a 12-worker elm-test run: median wall time drops from 83s to 31s (-62%) with all 70 tests still passing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
toTestevaluatesinstructionsToState instructionseagerly while constructing theTesttree, only to readstate.testName. Under elm-test's parallel runner, every worker process forces the whole test tree on startup, so each of the N workers re-runs the full E2E simulation for every test — even though each worker only reports results for its 1/N partition.This PR defers the simulation to inside the
Test.testlambda so that each worker only simulates the tests it actually runs. The test name is read via a new privateextractTestNamehelper that walks theEndToEndTestchain without applying anyNextStep/AndThenfunctions (cheap pattern unwrapping only).Diff (essentials)
Impact
Measured on a real-world Lamdera app with 14 E2E tests + 56 unit tests, 12 elm-test workers, cold runs on macOS arm64:
That is −62 % wall time, −85 % cumulative test duration, with all tests still passing.
The before-numbers also explain previously-mysterious CPU figures: cumulative user CPU was ~580 s for an 83 s wall, i.e. each of the 12 workers was burning ~50 s of CPU re-doing simulation work it would never report. After the patch, workers only simulate their assigned tests.
Test plan
elm-format src/Effect/Test.elm(no diff)lamdera makesucceeds with no warningselm-reviewpasses (no new errors; 4 pre-existing suppressions untouched)elm-test --compiler=lamdera— same passes/failures as before, faster wall timeBackwards compatibility
No public API change. The
EndToEndTestconstructors are not exposed from the module, the newextractTestNameis private, andtoTest's type signature is unchanged. Test names still display identically — they're just read via constructor unwrapping rather than running the simulation.Notes
The test name walk reads the
testNamefrom the originalStart stateset bystart. There is currently no public API inEffect.Testthat renames a test mid-chain via aNextStep, so this is always the correct name. If a future API ever lets users rename tests via state updates,extractTestNamewould need to apply such updates — but that case doesn't exist today.