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

Avoid extraneous copy when _recving bytes through pipes in multiprocessing "connections" #96059

Open
igozali opened this issue Aug 17, 2022 · 2 comments
Labels
performance Performance or resource usage stdlib Python modules in the Lib dir

Comments

@igozali
Copy link

igozali commented Aug 17, 2022

Feature or enhancement

Avoid extraneous copy when _recving bytes through pipes in multiprocessing "connections"

Pitch

In the default IPC implementation of the multiprocessing module using pipes, the Connection._recv method is defined as follows:

def _recv(self, size, read=_read):
buf = io.BytesIO()
handle = self._handle
remaining = size
while remaining > 0:
chunk = read(handle, remaining)
n = len(chunk)
if n == 0:
if remaining == size:
raise EOFError
else:
raise OSError("got end of file during message")
buf.write(chunk)
remaining -= n
return buf

Seems like we can avoid a copy if we can read directly into a preallocated byte array? This would be beneficial if the bytes we're sending between processes are large. Another benefit here is that the buf.write() call above doesn't release the GIL, so in my workloads I've seen it causing stalls in another thread when trying to read from a multiprocessing.Queue instance.

Sample implementation that avoids the extraneous copy is shown below (bit untested). If it makes sense, I can submit a patch, but probably need some guidance to deal with the Windows case:

import io


def unix_readinto(fd, buf):
    # Thanks https://stackoverflow.com/q/13919006/1572989
    return io.FileIO(fd, closefd=False).readinto(buf)


class Connection(...):
    ...

    def _recv(self, size, readinto=unix_readinto):
        buf = io.BytesIO(initial_bytes=bytes(size))
        handle = self._handle
    
        mem = buf.getbuffer()
        total = 0
        while total != size:
            n = readinto(handle, mem[total:])
    
            if n == 0:
                if total == 0:
                    raise EOFError
                else:
                    raise OSError("got end of file during message")
    
            total += n
    
        return buf

Previous discussion

N/A

@igozali igozali added the type-feature A feature request or enhancement label Aug 17, 2022
@corona10
Copy link
Member

corona10 commented Aug 18, 2022

Hmm according to my benchmark, it makes slower on macOS.
python/pyperformance#228

Would you like to suggest your benchmark to prove your optimization?
46.2 ms +- 1.1 ms -> 51.0 ms +- 1.1 ms (your suggestion)

cc @mdboom

@igozali
Copy link
Author

igozali commented Aug 18, 2022

Thanks for the feedback! That's interesting and somewhat counterintuitive? I haven't had the chance to test the above much, just that it seemed to make sense theoretically that removing an extra copy would be good. Will try some things out on my side.

@corona10 corona10 added performance Performance or resource usage and removed type-feature A feature request or enhancement labels Aug 18, 2022
@iritkatriel iritkatriel added the stdlib Python modules in the Lib dir label Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance or resource usage stdlib Python modules in the Lib dir
3 participants