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

Use PipeChannels or FdStreams to make worker processes natively async #1783

Closed

Conversation

richardsheridan
Copy link
Contributor

@richardsheridan richardsheridan commented Oct 31, 2020

This PR uses #1782 to make the workers in #1781 more natively async. This is a separate PR because:

  • Worker processes #1781 is actually feature complete as far as I can tell even with the threads, which don't leak during cancellation.
  • It adds substantial platform-dependent complexity into the new WorkerProc classes which would previously have been hidden within multiprocessing.Pipe.
  • It needs to reach into the internals of the handle/fd holder objects to avoid the problems of TrioInternalError while using a PipeReceiveStream #1767.

However, it provides two attractive opportunities:

  • The pickler is exposed and available to be exchanged for something less picky to transfer objects between processes.
  • Avoiding the use of worker threads as much as possible (This speeds up certain microbenchmarks, and ideally it could also use a thread-free replacement for trio.lowlevel.WaitForSingleObject)

These commits should be straightforward to rebase on top of #1781 and #1782, if those are accepted. Until then, this PR can just serve as a manifesto for the other PRs.

@codecov
Copy link

codecov bot commented Oct 31, 2020

Codecov Report

Merging #1783 (4ba5c30) into master (f751897) will decrease coverage by 0.07%.
The diff coverage is 97.30%.

@@            Coverage Diff             @@
##           master    #1783      +/-   ##
==========================================
- Coverage   99.64%   99.57%   -0.08%     
==========================================
  Files         114      117       +3     
  Lines       14503    14932     +429     
  Branches     1105     1130      +25     
==========================================
+ Hits        14451    14868     +417     
- Misses         37       45       +8     
- Partials       15       19       +4     
Impacted Files Coverage Δ
trio/_worker_processes.py 92.10% <92.10%> (ø)
trio/__init__.py 100.00% <100.00%> (ø)
trio/_core/_windows_cffi.py 100.00% <100.00%> (ø)
trio/_windows_pipes.py 100.00% <100.00%> (ø)
trio/tests/test_windows_pipes.py 100.00% <100.00%> (ø)
trio/tests/test_worker_process.py 100.00% <100.00%> (ø)
trio/to_process.py 100.00% <100.00%> (ø)

@richardsheridan richardsheridan marked this pull request as draft November 9, 2020 14:11
@richardsheridan richardsheridan force-pushed the async_workers branch 2 times, most recently from 7f637d2 to 71b2b5e Compare November 17, 2020 14:39
@richardsheridan richardsheridan force-pushed the async_workers branch 2 times, most recently from af5a774 to 8f092f3 Compare November 24, 2020 15:17
@richardsheridan richardsheridan changed the title Use windows pipes to make worker process natively async Use PipeChannels or FdStreams to make worker processes natively async Nov 24, 2020
@richardsheridan
Copy link
Contributor Author

Closing in favor of trio-parallel. Feel free to reopen someday though!

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.

1 participant