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

Remove race conditions in parallel builds #6729

Closed
vicuna opened this Issue Dec 21, 2014 · 9 comments

Comments

Projects
None yet
1 participant
@vicuna
Copy link
Collaborator

vicuna commented Dec 21, 2014

Original bug ID: 6729
Reporter: @whitequark
Assigned to: @whitequark
Status: closed (set by @xavierleroy on 2016-12-07T10:47:01Z)
Resolution: fixed
Priority: normal
Severity: minor
Fixed in version: 4.02.2+dev / +rc1
Category: configure and build/install
Monitored by: @gasche @hcarty

Bug description

There are a few annoying race conditions that break builds with high -j values.

File attachments

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Dec 21, 2014

Comment author: @whitequark

@gasche: One of the race conditions is present in interaction between make's built-in functionality that removes intermediate files referred to by .SUFFIXES: and explicit rm's. I rewrote the rules and now make -C byterun -j1000 always finishes successfully. Ditto for asmrun.

There are still more races in world, though.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Dec 21, 2014

Comment author: @gasche

Maybe we could apply the same treatment to the Makefile.nt (which is run under Windows) to avoid divergence? byerun/Makefile.nt does not have pic rules, but asmrun/Makefile.nt has some suffix rules left.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Dec 21, 2014

Comment author: @whitequark

I've updated the patch.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Dec 22, 2014

Comment author: @damiendoligez

FTR, it's not only for race condition, but it will also make my life easier when debugging the runtime.

But: could you minimise the patch? This does three things:

  1. change the build instructions to remove the race conditions
  2. change the rule style from ".c.o:" to "%.o : %.c"
  3. reorder some rules in asmrun/Makefile

I think (3) is really not needed, and I'd be more comfortable with the patch if it didn't do (2) at the same time, unless (1) somehow depends on it.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Dec 22, 2014

Comment author: @whitequark

It does. The automatic removal performed by make is linked to the suffix being in the .SUFFIXES: list.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Dec 22, 2014

Comment author: @gasche

On the other hand, the gmake documentation describes suffix rules as obsolete ( https://www.gnu.org/software/make/manual/html_node/Suffix-Rules.html ) and advises to replace them with pattern rules. As someone that doesn't know much about make, I find pattern rules much easier to read (I would never figure what ".s.p.o:" means).

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Dec 22, 2014

Comment author: @whitequark

Patch altered to not reorder rules in asmrun/Makefile.

@gasche: I've also removed the only other instance of .SUFFIXES: used to build C files. The build system also has widespread use of .SUFFIXES: to build ML files, but fixing that would be a part of much larger refactoring effort.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Dec 22, 2014

Comment author: @damiendoligez

OK, then.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Dec 27, 2014

Comment author: @gasche

Merged in trunk and 4.02.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.