Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a global lock around the out/ folder to allow a single mill command to run at a time #3519

Open
lihaoyi opened this issue Sep 12, 2024 · 3 comments

Comments

@lihaoyi
Copy link
Member

lihaoyi commented Sep 12, 2024

Follow up to #3397

Mill does not have a proper concurrency model, so running multiple mill commands in parallel can result in all sorts of crashes and misbehaviors. This is especially true because IDEs tend to run Mill on their own when trying to import the project.

Here are some issues I suspect are related:

#3243 was added to address parallel evaluations within the same process via BSP, but parallel evaluations across multiple processes remains a problem.

As a first pass, we should put all Mill evaluations between a global filesystem lock. This is the approach Bazel takes, and while not perfect, it works acceptably even for very large codebases and very large engineering organizations. We can add an optional --no-global-lock flag to disable this on an opt-in basis, but given the ongoing issues we need to go for correctness first by default, which means a global lock. The opt-out flag should be enough to handle edge cases where people really need the parallelism.

Implementing #3144 and moving the IDE work to a separate folder .bsp/out will help mitigate the most common concurrency issue of someone running 1 terminal and 1 IDE, at the cost of a bit of wasted computation (hopefully not too much)

We can figure out a finer-grained concurrency story later to try and optimize this, but I expect that will take a lot more time with no guarantee of success, so we should go with the global lock first

@lihaoyi
Copy link
Member Author

lihaoyi commented Sep 12, 2024

cc @alexarchambault

@alexarchambault
Copy link
Contributor

Fixed by #3599 I think

@lihaoyi
Copy link
Member Author

lihaoyi commented Oct 10, 2024

Let's merge this one too then we can close this #3683

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

No branches or pull requests

2 participants