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

Enhanced new build_many to use on all platforms #28356

Closed
embray opened this issue Aug 15, 2019 · 10 comments
Closed

Enhanced new build_many to use on all platforms #28356

embray opened this issue Aug 15, 2019 · 10 comments

Comments

@embray
Copy link
Contributor

embray commented Aug 15, 2019

In #27490 I hacked together a replacement for the sage_setup.docbuild.build_many function, which implements (blocking) parallel map() of sorts, which solved some problems with using multiprocessing.Pool.map that stems from its use of threads to fork new processes.

That solved a problem that was specific to older versions of Cygwin. However, there is a similar problem, which affects all platforms, with PARI built with multi-threading support: #26608.

Although the PARI problem begs a more complete solution, at least for the docbuild we can get around it by using the build_many from #27490.

This ticket makes some fixes and enhancements to build_many, so that it can also return a result from the function it runs. For the docbuild this feature is not strictly needed, except for the fact that it can also be used (as in Pool.map) to raise any exception that occurs in one of the worker processes. Thus, it's closer in functionality, at least for the purposes of the docbuild, to Pool.map.

CC: @antonio-rojas @jdemeyer @kiwifb @dimpase @saraedum

Component: build

Keywords: docbuild parallel pari

Author: Erik Bray

Branch/Commit: e8d26b6

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/28356

@embray embray added this to the sage-8.9 milestone Aug 15, 2019
@slel
Copy link
Member

slel commented Aug 25, 2019

comment:2

Branch does not seem to merge.

@embray
Copy link
Contributor Author

embray commented Aug 26, 2019

comment:3

Well it used to until a day ago...

@dimpase
Copy link
Member

dimpase commented Aug 28, 2019

Changed branch from u/embray/docbuild/build-many-2 to public/docbuild/build-many-2

@dimpase
Copy link
Member

dimpase commented Aug 28, 2019

comment:4

rebased branch


New commits:

65c7563Enhance the new build_many to actually return results (including exceptions).
e8d26b6Use new build_many on all platforms.

@dimpase
Copy link
Member

dimpase commented Aug 28, 2019

Changed commit from bd92f13 to e8d26b6

@embray
Copy link
Contributor Author

embray commented Aug 29, 2019

comment:5

Anyone want to go ahead and try using this?

@antonio-rojas
Copy link
Contributor

comment:6

Built docs on Arch against system packages including threaded pari, can confirm it no longer segfaults.

@dimpase
Copy link
Member

dimpase commented Aug 30, 2019

comment:7

looks good to me.

@dimpase
Copy link
Member

dimpase commented Aug 30, 2019

Reviewer: Dima Pasechnik

@vbraun
Copy link
Member

vbraun commented Sep 5, 2019

Changed branch from public/docbuild/build-many-2 to e8d26b6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants