[issue35813] shared memory construct to avoid need for serialization between processes

Change by Xavier de Gaye : -- nosy: -xdegaye ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.py

[issue35813] shared memory construct to avoid need for serialization between processes

Pam McA'Nulty added the comment: Reviewing docs and am writing a queue implementation for usability testing. I think ShareableList needs to support `close()` and `unlink()` directly. The `.shm` attribute should be considered an _implementation_ detail, and not be exposed. -- nosy:

[issue35813] shared memory construct to avoid need for serialization between processes

Xavier de Gaye added the comment: After changeset e895de3e7f3cc2f7213b87621cfe9812ea4343f0, test_all fails on platforms that lack the _posixshmem extension module (Android for example): == FAIL: test_all (test.test___all__.Al

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: @Giampaolo: The docstring in the shared_memory module currently marks the API as experimental. (You read my mind...) I will start a new PR where we can work on the better-integration-into-the-larger-multiprocessing-docs and add comments there. -- __

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: New changeset e895de3e7f3cc2f7213b87621cfe9812ea4343f0 by Davin Potts in branch 'master': bpo-35813: Tests and docs for shared_memory (#11816) https://github.com/python/cpython/commit/e895de3e7f3cc2f7213b87621cfe9812ea4343f0 -- ___

[issue35813] shared memory construct to avoid need for serialization between processes

Giampaolo Rodola' added the comment: IMHO given the size of the change and how quickly this evolved I would probably feel safer to mark the API as experimental (no pun intended, this is great work), at least in alpha2. I believe we now have a good explanation in the docs but it still needs t

[issue35813] shared memory construct to avoid need for serialization between processes

Raymond Hettinger added the comment: Any objections to getting this patch applied today so that it will be in the second alpha? Of course, further adjustments can still be made afterwards if needed. -- ___ Python tracker

[issue35813] shared memory construct to avoid need for serialization between processes

Giampaolo Rodola' added the comment: > Side note: I’m the author of the sysv_ipc package you found, as well as the > posix_ipc package. Sorry, I didn't realize it was you. > Not true. A manager started by one process can be connected to by another > process that is not a child. This is cov

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: > FWIW I bumped into this lib: http://semanchuk.com/philip/sysv_ipc/ The author of that lib, Philip Semanchuk, is one of the people participating in this effort -- he has posted above in msg334934 here on b.p.o. and has helped review the PR in GH-11816. He is

[issue35813] shared memory construct to avoid need for serialization between processes

Philip Semanchuk added the comment: > On Feb 23, 2019, at 10:40 AM, Giampaolo Rodola' > wrote: > > > Giampaolo Rodola' added the comment: > >> We are consciously choosing to not support an atomic "create or attach". >> This significantly simplifies the API and avoids the valid concerns r

[issue35813] shared memory construct to avoid need for serialization between processes

Giampaolo Rodola' added the comment: > We are consciously choosing to not support an atomic "create or attach". > This significantly simplifies the API and avoids the valid concerns raised > around user confusion relating to that behavior (including the use of > different specified 'size' v

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: > Code looks much better now. I'm still not convinced > "SharedMemory(name=None, create=False, size=0)" is the best API. > How are you supposed to "create or attach" atomically? We are consciously choosing to not support an atomic "create or attach". This sign

[issue35813] shared memory construct to avoid need for serialization between processes

Giampaolo Rodola' added the comment: Also, there is no way to delete/unwrap memory without using an existing SharedMemory instance, which is something we may not have on startup. Perhaps we should have a "shared_memory.unlink(name)" function similar to os.unlink() which simply calls C shm_un

[issue35813] shared memory construct to avoid need for serialization between processes

Giampaolo Rodola' added the comment: Code looks much better now. I'm still not convinced "SharedMemory(name=None, create=False, size=0)" is the best API. How are you supposed to "create or attach" atomically? You can do that with O_EXCL but as it stands this is not togglable via the API. Al

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: The simpler API is now implemented in GH-11816 as discussed previously. Notably: > * We go with this simpler API: SharedMemory(name=None, create=False, size=0) > * 'size' is ignored when create=False > * create=True acts like O_CREX and create=False only attach

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: > I think we need the "create with exclusive behavior" option, even > though we don't know how to implement it on Windows right now. A fix to avoid the potential race condition on Windows is now part of GH-11816. > To support 1 & 2, we could just have 'create'.

[issue35813] shared memory construct to avoid need for serialization between processes

Neil Schemenauer added the comment: Some thoughts on this API. I think we need the "create with exclusive behavior" option, even though we don't know how to implement it on Windows right now. To me, there are two cases when calling SharedMemory: 1) You want to create a new shared memory ob

[issue35813] shared memory construct to avoid need for serialization between processes

Giampaolo Rodola' added the comment: > It looks like it does skip calling ftruncate() if size is 0. From > posixshmem.c: Let me rephrase: are we forced to specify a value (aka call ftruncate()) on create ? If we are as I think, could size have a reasonable default value instead of 0? Basica

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: @giampaolo: > Also, what happens if you alter the size of an existing object with a smaller > value? Is the memory region overwritten? Attaching to an existing shared memory block with a size=N which is smaller than its allocated size (say it was created with

[issue35813] shared memory construct to avoid need for serialization between processes

Giampaolo Rodola' added the comment: As for *flags* argument "man shm_open" mentions 5 possible flags: - O_RDONLY, OR_RDWR: these can be served by existing *read_only* arg - O_CREAT, O_EXCL: dictates whether to raise error if file exists; could be served via *attach_if_exists* arg, if added,

[issue35813] shared memory construct to avoid need for serialization between processes

Giampaolo Rodola' added the comment: > The unlink() method is available on the SharedMemory class. No manager is > required. This is also captured in the docs. I missed that, sorry. >> 3) it seems "size" kwarg cannot be zero (current default) > From the docs: When attaching to an existing

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: @giampaolo: > 1) As for SharedMemoryManager, I believe it should live in > multiprocessing.managers, not shared_memory.py. I am happy to see it live in multiprocessing.managers so long as we can provide a clean way of handling what happens on a platform where

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: @giampaolo: > Maybe something like this instead? > SharedMemory(name=None, attach_if_exists=False, size=0) I think this misses the use case when wanting to ensure we only attach to an existing shared memory block and if it does not exist, we should raise a

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: @giampaolo: > 1) it seems SharedMemory.close() does not destroy the memory region > (I'm able to re-attach to it via name). If I'm not mistaken only > the manager can do that. Correct, close() does not and should not destroy the memory region because other proc

[issue35813] shared memory construct to avoid need for serialization between processes

Giampaolo Rodola' added the comment: Hopefully my last iteration: =) 1) As for SharedMemoryManager, I believe it should live in multiprocessing.managers, not shared_memory.py. It's a subclass of Manager and behaves like a manager (start(), shutdown(), get_server(), etc.), so IMO that's its

[issue35813] shared memory construct to avoid need for serialization between processes

Giampaolo Rodola' added the comment: > Because shared memory blocks are not "owned" by a single process... > [...] > I propose a simpler, consistent-across-platforms API like: > SharedMemory(name=None, create=False, size=0) Maybe something like this instead? SharedMemory(name=None,

[issue35813] shared memory construct to avoid need for serialization between processes

Giampaolo Rodola' added the comment: > Because shared memory blocks are not "owned" by a single process, they are > not destroyed/freed when a process exits. 1) it seems SharedMemory.close() does not destroy the memory region (I'm able to re-attach to it via name). If I'm not mistaken only

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: Regarding the API of the SharedMemory class, its flags, mode, and read_only parameters are not universally enforced or simply not implemented on all platforms that offer POSIX Shared Memory or Windows Named Shared Memory. A simplified API for the SharedMemory

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: These questions (originally asked in comments on GH-11816) seemed more appropriate to discuss here: Why should the user want to use `SharedMemory` directly? Why not just go through the manager? Also, perhaps a naive question: don't you _always_ need

[issue35813] shared memory construct to avoid need for serialization between processes

Antoine Pitrou added the comment: > SharedMemoryManager does not subclass SyncManager but it did previously. Ah, right. Then it's ok to change the tests IMO. -- ___ Python tracker _

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: @Antoine: SharedMemoryManager does not subclass SyncManager but it did previously. This is the source of the confusion. SharedMemoryManager subclasses BaseManager which does not provide Value, Array, list, dict, etc. Agreed that the manager facility does not

[issue35813] shared memory construct to avoid need for serialization between processes

Antoine Pitrou added the comment: Davin: > This is my fault because I altered SharedMemoryManager to no longer support > functionality from SyncManager that I thought could be confusing to include. > I am just now discovering this and am not immediately sure if simply removing > the Shared

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: @giampaolo.rodola: It definitely helps. Conceptually, SyncManager provides "distributed shared memory" where lists, dicts, etc. are held in memory by one process but may be accessed remotely from another via a Proxy Object. Mutating a dict from one process re

[issue35813] shared memory construct to avoid need for serialization between processes

Raymond Hettinger added the comment: FWIW, I read the draft docs and found them to be at the right level throughout. I definitely wouldn't want anything more terse. -- ___ Python tracker ___

[issue35813] shared memory construct to avoid need for serialization between processes

Giampaolo Rodola' added the comment: I submitted an initial review / comments in the PR. I think this is good for a first iteration in order to understand what APIs to expose publicly (also, keep in mind I may not have a full picture of how this is intended to be used precisely). As for the

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: @terry.reedy and @ronaldoussoren: I have asked Van again to provide comments here clarifying the topics of (1) copyright notices and (2) requiring the BSD-licensed-work's author to sign a contributor agreement. Specifically regarding the appearance of __copyrig

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: @giampaolo.rodola: Your patch from 3 days ago in issue35917 included additional tests around the SharedMemoryManager which are now causing test failures in my new PR. This is my fault because I altered SharedMemoryManager to no longer support functionality fro

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: Docs and tests are now available in a new PR. I have stayed focused on getting these docs and tests to everyone without delay but that means I have not yet had an opportunity to respond to the helpful comments, thoughtful questions, and threads that have poppe

[issue35813] shared memory construct to avoid need for serialization between processes

Change by Davin Potts : -- pull_requests: +11834 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail

[issue35813] shared memory construct to avoid need for serialization between processes

Change by Davin Potts : -- pull_requests: +11834, 11835, 11836 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35813] shared memory construct to avoid need for serialization between processes

Change by Davin Potts : -- pull_requests: +11834, 11835 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https

[issue35813] shared memory construct to avoid need for serialization between processes

Antoine Pitrou added the comment: (also, OSError will automatically convert to more specific subclasses for some errnos, see PEP 3151: >>> raise OSError(errno.ENOENT, "foobar") >>>

[issue35813] shared memory construct to avoid need for serialization between processes

Neil Schemenauer added the comment: I didn't finish reviewing completely yet but here are some comments. I think the random filename generation can be pulled out of _posixshmem. Make it require that a filename is passed into it. That moves a fair bit of complexity out of C and into Python. I

[issue35813] shared memory construct to avoid need for serialization between processes

Giampaolo Rodola' added the comment: Also, for completeness (since discussion is getting split), please see my proposal to move SharedMemoryManager and SharedMemoryServer into multiprocessing.managers namespace and rename shared_memory.py to _shared_memory.py: https://mail.python.org/piperma

[issue35813] shared memory construct to avoid need for serialization between processes

Giampaolo Rodola' added the comment: Unit-tests at https://bugs.python.org/issue35917. -- nosy: +giampaolo.rodola ___ Python tracker ___ __

[issue35813] shared memory construct to avoid need for serialization between processes

Terry J. Reedy added the comment: I would prefer that we be consistent. In any case, I think you should be added to Misc/ACKS in the PR. -- ___ Python tracker ___ __

[issue35813] shared memory construct to avoid need for serialization between processes

Philip Semanchuk added the comment: Hi all, I'm the author of `posix_ipc` on which some of this code is based. I'd be happy to sign a contributor agreement in order to erase any concerns on that front. -- nosy: +osvenskan ___ Python tracker

[issue35813] shared memory construct to avoid need for serialization between processes

Change by Stefan Krah : -- nosy: +skrah ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.o

[issue35813] shared memory construct to avoid need for serialization between processes

Change by pmpp : -- nosy: +pmpp ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailm

[issue35813] shared memory construct to avoid need for serialization between processes

Eric Snow added the comment: FTR, I've had a number of extensive conversations with Davin (over the last year and a half) about this feature. His choices seemed reasonable (caveat: I'm not an expert on multiprocessing) and the feature seemed desirable. I did not review his PR. --

[issue35813] shared memory construct to avoid need for serialization between processes

Antoine Pitrou added the comment: Also, I will want to review this as well. Since Alpha 2 is scheduled for 2019-02-24, and this is a sizable piece of work, I would prefer if a full PR was available, say, before 2019-02-15. -- ___ Python tracker

[issue35813] shared memory construct to avoid need for serialization between processes

Ronald Oussoren added the comment: Please also address my review comments. -- ___ Python tracker ___ ___ Python-bugs-list mailing l

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: @lukasz.langa: Missing tests and documentation will be in by alpha2. -- ___ Python tracker ___ ___ P

[issue35813] shared memory construct to avoid need for serialization between processes

Łukasz Langa added the comment: @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 me

[issue35813] shared memory construct to avoid need for serialization between processes

Ronald Oussoren added the comment: 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 Git

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: 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 encoura

[issue35813] shared memory construct to avoid need for serialization between processes

Terry J. Reedy added the comment: 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 copyr

[issue35813] shared memory construct to avoid need for serialization between processes

Change by Ned Deily : -- nosy: -ned.deily ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.pytho

[issue35813] shared memory construct to avoid need for serialization between processes

Antoine Pitrou added the comment: 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). -- ___ Python tracker

[issue35813] shared memory construct to avoid need for serialization between processes

Ronald Oussoren added the comment: FWIW I agree with reverting this pull request, the work is clearly not finished yet. -- ___ Python tracker ___

[issue35813] shared memory construct to avoid need for serialization between processes

Antoine Pitrou added the comment: 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 as

[issue35813] shared memory construct to avoid need for serialization between processes

Ronald Oussoren added the comment: Was this merged too soon? This is new functionality without any docs and tests. I've also left review comments on the pull request. -- nosy: +ronaldoussoren ___ Python tracker

[issue35813] shared memory construct to avoid need for serialization between processes

Davin Potts added the comment: New changeset e5ef45b8f519a9be9965590e1a0a587ff584c180 by Davin Potts in branch 'master': bpo-35813: Added shared_memory submodule of multiprocessing. (#11664) https://github.com/python/cpython/commit/e5ef45b8f519a9be9965590e1a0a587ff584c180 -- __

[issue35813] shared memory construct to avoid need for serialization between processes

Change by Davin Potts : -- keywords: +patch, patch pull_requests: +11470, 11471 stage: -> patch review ___ Python tracker ___ ___ P

[issue35813] shared memory construct to avoid need for serialization between processes

Change by Davin Potts : -- keywords: +patch, patch, patch pull_requests: +11470, 11471, 11472 stage: -> patch review ___ Python tracker ___ ___

[issue35813] shared memory construct to avoid need for serialization between processes

Change by Davin Potts : -- keywords: +patch pull_requests: +11470 stage: -> patch review ___ Python tracker ___ ___ Python-bugs-lis

[issue35813] shared memory construct to avoid need for serialization between processes

New submission from Davin Potts : 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