Skip to content

Conversation

@rocallahan
Copy link
Contributor

If your work is part of a larger effort, please discuss your general plans on Discourse first to align your vision with maintainers.

This is following up on discussion where we agreed to pursue making RTLIL safe for multithreaded read-only access.

What are the reasons/motivation for this change?

Yosys tests already run tests in parallel, potentially up to the point where the number of running tests equals the number of CPUs if the user uses make -j test. If each test spawns a number of threads equal to the number of CPUs, the total number of threads could be quadratic in the number of CPUs, which could be bad.

Instead limit each test to 4 threads.

This change also makes it easy to do scalability performance tests without using thread affinity tricks like taskset.

Copy link
Collaborator

@widlarizer widlarizer left a comment

Choose a reason for hiding this comment

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

I'm on board with the idea, but the variable seems to be applied in the wrong spot. Our testing automation is definitely suboptimal and we're making it a focus to figure out how to make it less painful, but right now, the variable seems to have to be applied in a couple of places, or in the top-level Makefile

@widlarizer
Copy link
Collaborator

@rocallahan
Copy link
Contributor Author

Yeah we could do that but I don't think that complexity would benefit us much. In fact arguably we don't even need this PR; to keep small examples fast each pass that uses parallelism will have to throttle the number of threads used, and in particular, for very small examples you have to just stick to the main thread. So pretty much all tests will end up using only one thread anyway. But this is simple and a nice backstop in case that doesn't happen.

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.

2 participants