Skip to content

imgmath: Avoid parallel workers depth write race #13457

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jschueller
Copy link
Contributor

@jschueller jschueller commented Mar 28, 2025

Ensure parallel workers do not try to write the image depth multiple times to achieve reproducible builds without -j1.

Typically some of my svg files randomly ended up like this:

...
<!-- DEPTH=0 -->
<!-- DEPTH=0 -->

So images were not the same and sometimes the depth was incorrect too.

Introduces a new dependency to the filelock module though.

My project openturns has a pretty big doc with ~8k math formulas and I didnt notice any slowdown with this (-j8).

@jschueller jschueller force-pushed the depth branch 6 times, most recently from 72b29dd to fe3ea1f Compare March 28, 2025 08:55
@jschueller jschueller marked this pull request as draft March 28, 2025 09:26
@jschueller jschueller force-pushed the depth branch 3 times, most recently from 39a111d to ffb1fd7 Compare March 28, 2025 13:16
@jschueller jschueller marked this pull request as ready for review March 28, 2025 15:31
@AA-Turner
Copy link
Member

Can we do this without a new dependency?

@jschueller
Copy link
Contributor Author

jschueller commented Mar 28, 2025

I tried to avoid that by writing the depth to a separate file, but I just circled backed to the same kind of race issue, unless you have other ideas ? else there is os.lockf but that's unix-only.

Maybe it can become optional, ie try to load it else continue.

@jschueller jschueller force-pushed the depth branch 7 times, most recently from bbfd65f to 61c0105 Compare April 27, 2025 08:01
@jschueller
Copy link
Contributor Author

@AA-Turner I made the filelock module optional, what do you think ?

@jschueller jschueller force-pushed the depth branch 2 times, most recently from 1468ce9 to f1062c9 Compare April 28, 2025 18:09
Ensure parallel workers do not try to write the image depth
multiple times to achieve reproducible builds.

Typically my svg files ended up like this:
```
...
<!-- DEPTH=0 -->
<!-- DEPTH=0 -->
```
So images were not the same and sometimes the depth was incorrect too.

Introduces a new dependency to the filelock module.
Instead of mandatory
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