search

foundry-rs / foundry

Foundry is a blazing fast, portable and modular toolkit for Ethereum application development written in Rust.
https://getfoundry.sh
Apache License 2.0
8.15k stars 1.69k forks source link

`forge test` doesn't utilize all available threads for fuzzing/invariants #8898

Open Philogy opened 5 days ago

Philogy commented 5 days ago

Component

Forge

Have you ensured that all of these are up to date?

What version of Foundry are you on?

forge 0.2.0 (03ea54c 2024-09-18T16:31:08.207328652Z)

What command(s) is the bug in?

forge test

Operating System

None

Describe the bug

Forge testing is multi-threaded but only on a per test level, for instance if I have 15 different fuzz runs and 224 available threads foundry will assign 15/224 threads with one test each, however some tests take much longer to run than others. Furthermore individual runs of the same test are completely independent. I'd expect different runs to get scheduled to different threads but foundry doesn't seem to do that, leaving a lot of performance on the table.

grandizzy commented 4 days ago

good point, we're currently limited by proptest test runner but should consider changing this post 1.0 release

Philogy commented 4 days ago

good point, we're currently limited by proptest test runner but should consider changing this post 1.0 release

This would be an amazing feature to have, would mean an 8-224x speedup depending on the machine I run the tests on

Philogy commented 4 days ago

Could there be a temporary workaround where foundry duplicates a given test or invariant for you by the number of available threads and then does total_runs / N runs on each?

I can do it manually but it gets a but unwieldy: image

also I know either have to do this for all my fuzz tests or run different suites with different profiles

Philogy commented 4 days ago

@zerosnacks not quite sure I understand how this would be a breaking change or what does T-breaking mean in this context?

How the tests are run should be abstracted away from the end user, no?

zerosnacks commented 4 days ago

You are correct, we use T-breaking as an internal tag to indicate it would likely require making a breaking change and we should decide whether to include it before or after 1.0. Moving away from proptest would be a huge change but indeed should not be experienced as breaking by the end-user.

Philogy commented 4 days ago

I see, good to know thx