ruff icon indicating copy to clipboard operation
ruff copied to clipboard

[red-knot] Refactor `program.check` scheduling

Open MichaReiser opened this issue 1 year ago • 1 comments

Summary

This PR refactors program.check, specifically the part around custom schedulers.

What I found awkward in the old implementation is that check owned the orchestration. Normally, that's something the scheduler wants to take care of. Giving that control to the scheduler gives it more freedom how to do the scheduling.

Inversing control further has the advantage that the SingleThreadedScheduler can use lock-free data structures exclusively (it had to communicate with the main thread before by using a channel). The SingleThreadedScheduler implementation now also looks roughly how I would have wanted to write it: a single queue with all the tasks and a loop that processes the tasks until done.

I don't know if this is fundamentally better, but it feels less around the corner to me (although, it required some going around the corner to make it work :laughing:)

This PR also fixes an issue in the MultithreadedExecutor where it would dead lock when the open files is larger than the available parallelism. It also fixes that it uses the thread pool size (current_num_threads) over the system supported max number of threads (max_num_threads).

Test Plan

I used the slow lint to test that cancellation works correctly.

Alternative design

I think it would still be possible for program.check to accept an impl Executor and we may want to do this in the future (and should only require making some methods public). For now, having an enum seems sufficient.

MichaReiser avatar Apr 29 '24 12:04 MichaReiser

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

github-actions[bot] avatar Apr 29 '24 12:04 github-actions[bot]