shared memory construct to avoid need for serialization between processes

[applio]

BPO	35813
Nosy	@brettcannon, @nascheme, @rhettinger, @terryjreedy, @ronaldoussoren, @pitrou, @osvenskan, @giampaolo, @skrah, @pmp-p, @ambv, @ericsnowcurrently, @1st1, @applio
PRs	bpo-35813: Added shared_memory submodule of multiprocessing. #11664 bpo-35813: Added shared_memory submodule of multiprocessing. #11664 bpo-35813: Added shared_memory submodule of multiprocessing. #11664 bpo-35813: Tests and docs for shared_memory #11816 bpo-35813: Tests and docs for shared_memory #11816

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/applio'
closed_at = None
created_at = <Date 2019-01-24.04:02:05.382>
labels = ['3.8', 'type-feature', 'library']
title = 'shared memory construct to avoid need for serialization between processes'
updated_at = <Date 2019-12-10.07:52:25.564>
user = 'https://github.com/applio'

bugs.python.org fields:

activity = <Date 2019-12-10.07:52:25.564>
actor = 'xdegaye'
assignee = 'davin'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2019-01-24.04:02:05.382>
creator = 'davin'
dependencies = []
files = []
hgrepos = []
issue_num = 35813
keywords = ['patch', 'patch', 'patch']
message_count = 57.0
messages = ['334278', '334737', '334789', '334797', '334798', '334799', '334805', '334806', '334809', '334817', '334820', '334821', '334822', '334835', '334934', '334945', '334957', '334963', '335030', '335032', '335194', '335195', '335262', '335273', '335282', '335284', '335360', '335362', '335366', '335622', '335660', '335682', '335689', '335694', '335696', '335697', '335700', '335711', '335715', '335719', '335731', '335789', '335797', '336182', '336294', '336298', '336336', '336383', '336386', '336388', '336395', '336400', '336423', '336429', '336430', '338180', '341920']
nosy_count = 15.0
nosy_names = ['brett.cannon', 'nascheme', 'rhettinger', 'terry.reedy', 'ronaldoussoren', 'pitrou', 'osvenskan', 'giampaolo.rodola', 'skrah', 'pmpp', 'lukasz.langa', 'eric.snow', 'Pam.McANulty', 'yselivanov', 'davin']
pr_nums = ['11664', '11664', '11664', '11816', '11816']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue35813'
versions = ['Python 3.8']

[applio]

A facility for using shared memory would permit direct, zero-copy access to data across distinct processes (especially when created via multiprocessing) without the need for serialization, thus eliminating the primary performance bottleneck in the most common use cases for multiprocessing.

Currently, multiprocessing communicates data from one process to another by first serializing it (by default via pickle) on the sender's end then de-serializing it on the receiver's end. Because distinct processes possess their own process memory space, no data in memory is common across processes and thus any information to be shared must be communicated over a socket/pipe/other mechanism. Serialization via tools like pickle is convenient especially when supporting processes on physically distinct hardware with potentially different architectures (which multiprocessing does also support). Such serialization is wasteful and potentially unnecessary when multiple multiprocessing.Process instances are running on the same machine. The cost of this serialization is believed to be a non-trivial drag on performance when using multiprocessing on multi-core and/or SMP machines.

While not a new concept (System V Shared Memory has been around for quite some time), the proliferation of support for shared memory segments on modern operating systems (Windows, Linux, *BSDs, and more) provides a means for exposing a consistent interface and api to a shared memory construct usable across platforms despite technical differences in the underlying implementation details of POSIX shared memory versus Native Shared Memory (Windows).

For further reading/reference: Tools such as the posix_ipc module have provided fairly mature apis around POSIX shared memory and seen use in other projects. The "shared-array", "shared_ndarray", and "sharedmem-numpy" packages all have interesting implementations for exposing NumPy arrays via shared memory segments. PostgreSQL has a consistent internal API for offering shared memory across Windows/Unix platforms based on System V, enabling use on NetBSD/OpenBSD before those platforms supported POSIX shared memory.

At least initially, objects which support the buffer protocol can be most readily shared across processes via shared memory. From a design standpoint, the use of a Manager instance is likely recommended to enforce access rules in different processes via proxy objects as well as cleanup of shared memory segments once an object is no longer referenced. The documentation around multiprocessing's existing sharedctypes submodule (which uses a single memory segment through the heap submodule with its own memory management implementation to "malloc" space for allowed ctypes and then "free" that space when no longer used, recycling it for use again from the shared memory segment) will need to be updated to avoid confusion over concepts.

Ultimately, the primary motivation is to provide a path for better parallel execution performance by eliminating the need to transmit data between distinct processes on a single system (not for use in distributed memory architectures). Secondary use cases have been suggested including a means for sharing data across concurrent Python interactive shells, potential use with subinterpreters, and other traditional uses for shared memory since the first introduction of System V Shared Memory onwards.

[applio]

New changeset e5ef45b by Davin Potts in branch 'master':
bpo-35813: Added shared_memory submodule of multiprocessing. (bpo-11664)
e5ef45b

[ronaldoussoren]

Was this merged too soon? This is new functionality without any docs and tests.

I've also left review comments on the pull request.

[pitrou]

Yes, I think this was merged too soon. Davin, the convention is that we only merge completed work, not some work-in-progress.

You already did this once in https://bugs.python.org/issue28053 . You still haven't completed that piece of work.

I'm going to ask for this to be reverted.

[ronaldoussoren]

FWIW I agree with reverting this pull request, the work is clearly not finished yet.

[pitrou]

And to make things clear, I'm not saying anything about the functionality. Easier shared memory + multiprocessing is definitely an interesting endeavour (and a non-trivial one).

[terryjreedy]

I think there is also a license problem. posixshmem.c contains "Copyright 2012 Philip Semanchuk, 2018-2019 Davin Potts"

Ronald commented "The only other files with a copyright attribute are parser, optparse and platform. I'd prefer to avoid adding new copyright attributes to stdlib modules."

(turtle.py has a problematic copyright notice in the docstring.)

I think we definitely should not add new copyright notices.

Copyright notices are redundant: all contributors retain copyright in their contributions.

Copyright notices are deceptive: 1. since they are rare, they imply that there is something special about a particular module and the listed authors; 2. even if a module were special, the notice becomes obsolete as soon as anyone else contributes to the module.

Copyright notices are not needed: contributors authorize PSF to distribute the collective work under a rather liberal license. If anyone want to make a use of Python code not covered by that license, and wants to bypass PSF, they would have to look at git log and and git blame to find the relevant contributors.

In this case, part of the work is attributed to Philip Semanchuk as a current copyright owner. According to https://bugs.python.org/user2567, he has not signed the contributor agreement, so his work should not have been merged until he has. Even if he had, he would have to specifically agree to his work being contributed. Sorry to be a grinch, but aside from anything else, I think this should be reverted until the legal question is clarified.

Some of this might need discussion on pydev.

[applio]

This work is the result of ~1.5 years of development effort, much of it accomplished at the last two core dev sprints. The code behind it has been stable since September 2018 and tested as an independently installable package by multiple people.

I was encouraged by Lukasz, Yury, and others to check in this code early, not waiting for tests and docs, in order to both solicit more feedback and provide for broader testing. I understand that doing such a thing is not at all a novelty. Thankfully it is doing that -- I hope that feedback remains constructive and supportive.

There are some tests to be found in a branch (enh-tests-shmem) of github.com/applio/cpython which I think should become more comprehensive before inclusion. Temporarily deferring and not including them as part of the first alpha should reduce the complexity of that release.

Regarding the BSD license on the C code being adopted, my conversations with Brett and subsequently Van have not raised concerns, far from it -- there is a process which is being followed to the letter. If there are other reasons to object to the thoughtful adoption of code licensed like this one, that deserves a decoupled and larger discussion first.

[ronaldoussoren]

David,

I don't agree with merging at this point.

The lack of documentation makes it hard to ask for more feedback and broader testing, because people won't know how to use the library and provide constructive feedback (that's why my review comments on GitHub are mostly nitpicking).

Furthermore, distributing this as a package on PyPI is much better way to solicit further testing and feedback, because then it can be used right now instead of having to wait for a release. Releasing on PyPI is feasible for this new functionality because it is a standalone library.

It would be better to revert this changeset until it is much closer to finished and there has been time for better code review.

BTW. My comment about copyright on the pull request was about the __copyright__ attribute of the C extension, that is rarely used in the stdlib and is IMHO something to avoid.

[ambv]

@davin, in what time can you fill in the missing tests and documentation? If this is something you finish do before alpha2, I'm inclined to leave the change in.

As it stands, I missed the controversy yesterday as I was busy making my first release. So the merge *got released* in alpha1. I would prefer to fix the missing pieces forward instead of reverting and re-submitting which will only thrash blame and history at this point.

FTR, I do agree with Antoine, Ronald and others that in the future such big changes should be as close to their ready state at merge time.

[applio]

@lukasz.langa: Missing tests and documentation will be in by alpha2.