bpo-28053: Complete and fix custom reducers in multiprocessing.#9959
bpo-28053: Complete and fix custom reducers in multiprocessing.#9959pablogsal wants to merge 13 commits into
Conversation
bc59cf2 to
199325d
Compare
October 18, 2018 22:35
11763cf to
0745094
Compare
October 19, 2018 20:26
6ee544d to
e27684c
Compare
December 9, 2018 19:16
|
@pitrou Thank you very much for the review! I have simplified the API. Now setting a custom reducer looks like this: import multiprocessing
from multiprocessing.reduction import AbstractReducer, ForkingPickler
class ForkingPicklerProtocol2(ForkingPickler):
@classmethod
def dumps(cls, obj, pickle_protocol=2):
return super().dumps(obj, protocol=pickle_protocol)
class PickleProtocol2Reducer(AbstractReducer):
def get_pickler_class(self):
return ForkingPicklerProtocol2
multiprocessing.set_reducer(PickleProtocol2Reducer)I am making the interface a bit more strict, so |
Sorry, something went wrong.
8a68606 to
5e2fe4f
Compare
December 10, 2018 23:02
a16e157 to
856716c
Compare
May 27, 2019 17:44
|
@pitrou It took me a while but I have stabilized all tests and fixed some details on Windows. I have also added Listener and Client to the context so they also can benefit from custom reducers. Please, check my previous comment regarding some details. This patch is already very big and very very complex and when errors happen they are extremely obscure or platform dependent, so I apologize in advance if I miss something obvious, but I have too many spinning plates. Could you take another look? |
Sorry, something went wrong.
715733f to
7ee45e1
Compare
May 27, 2019 20:01
pitrou
left a comment
There was a problem hiding this comment.
Thanks for the update. It seems there are test failures on all 3 CI platforms...
Sorry, something went wrong.
4f09853 to
a6734a0
Compare
May 27, 2019 23:06
|
@pitrou There is some failures in Windows that I am investigating but I found a problem. In multiprocessing/context.py there is no way of passing down the current context to the Process class. The This is the only exception as any other class is defined in I don't know how to solve this, but basically tests like Trying to do something like: fails because As you are more used to the architecture of the multiprocessing module, do you see a way of solving this? If you don't see a way, I'm afraid that custom reducers per context cannot be implemented because of the way multiprocessing is architected. |
Sorry, something went wrong.
|
@pablogsal thanks a lot for putting this PR together, this is great work. This feature is very promising. Regarding the issue concerning the See proof of concept below @property
def Process(self):
if not self._custom_reduction_enabled:
# Ensure backward compatibility by returning a class when no
# custom reducer was specified
return _Process # ForkProcess for ForkContext, Process for BaseContext etc.
else:
return self.process_factory
def process_factory(self, *args, **kwargs):
p = Process(*args, **kwargs)
p._ctx = self.get_context()
return pMore complete implementation here: Gist showing a usage example and its behavior: What do you think? EDIT: Another question is whether or not we consider that cpython/Doc/library/multiprocessing.rst Lines 166 to 168 in 5a7132f If we want to enable |
Sorry, something went wrong.
|
Another topic that appears many times in this PR is the strange A good way IMO to re-establish the symmetry here would be to use actual Thus, we could add an optional |
Sorry, something went wrong.
This PR tries to complete and fix the implementation of the custom reducer classes in
multiprocessing.Important
I have marked the PR as
DO-NOT-MERGEbecause I have still several doubts about the previous implemented API, regarding theAbstractReducerbase class and the methods that the user needs to implement and how the rest of the library interacts withmultiprocessing.reducer. For example:I am not sure
multiprocessing.reducer.dumpsandmultiprocessing.reducer.registerare needed outside theForklingPicklerclass and how that interacts with the ABC.I am not sure the
AbstractReduceris implemented completely (there is no abstract methods marked).This PR is a draft implementation of the complete API, tests and documentation so we can discuss how to implement these correctly in a better way.
https://bugs.python.org/issue28053