Skip to content

Conversation

@stevenfontanella
Copy link
Member

@stevenfontanella stevenfontanella commented Dec 3, 2025

Reduces runtime from ~15 minutes to 1.5 minutes on my machine

Before:
image

After:
image

Failed test example (stdout and stderr isn't ordered with the exception unfortunately, but it's easy to re-run the particular test):
image

  • Run tests through a thread pool with os.cpu_count() * 2 threads
    • os.cpu_count() * 4 shows no benefit. os.cpu_count() or os.cpu_count() // 2 also show no regression in runtime, but might be worse for machines with less cores. There are currently 315 spec tests total for reference.
  • Prefixes round-trip file name tests with their test name to avoid clobbering the a.wasm / ab.wast files during tests
  • Add stdout and stderr params to functions that print so that lines can be captured by each thread and not interleaved
    • Note that we pass stdout as the stderr param in practice so that they are interleaved, otherwise all stdout lines and stderr lines will be outputted together in each test.

@stevenfontanella stevenfontanella force-pushed the multithread branch 3 times, most recently from 44c5834 to 62b613b Compare December 4, 2025 01:19
@stevenfontanella stevenfontanella changed the title Add multithreading for spec tests Run spec tests in parallel to reduce the execution time Dec 4, 2025
@stevenfontanella stevenfontanella marked this pull request as ready for review December 4, 2025 01:56
@tlively
Copy link
Member

tlively commented Dec 4, 2025

The alpine builder has been running for over three hours now, so I think there's a problem here. Looking at the log, it looks much more verbose than before because the stdout from the wasm-shell commands is being printed. Maybe fixing that will make the builder go faster?

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm % open comments

@stevenfontanella
Copy link
Member Author

Re:

The alpine builder has been running for over three hours now, so I think there's a problem here. Looking at the log, it looks much more verbose than before because the stdout from the wasm-shell commands is being printed. Maybe fixing that will make the builder go faster?

I changed support.run_command to not print the process's stdout and fixed the Alpine build (it's using a lower Python version and didn't have Queue.shutdown())

@stevenfontanella
Copy link
Member Author

I think the CI problem is that this member isn't initialized in the default constructor of SmallVector since std::array is an aggregate type. I'm not sure why we see the breakage here and not in main or in other architectures. I'll try to re-run for now and send a PR to fix separately.

CI link

@kripken
Copy link
Member

kripken commented Dec 5, 2025

About the std::array initialization error, we do disable that warning a few lines above for some archs, and maybe we just need to disable it in more. gcc does seem to have many such false positives...

@stevenfontanella stevenfontanella enabled auto-merge (squash) December 5, 2025 20:40
@stevenfontanella stevenfontanella merged commit 28e849b into main Dec 5, 2025
17 checks passed
@stevenfontanella stevenfontanella deleted the multithread branch December 5, 2025 21:10
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.

4 participants