-
-
Notifications
You must be signed in to change notification settings - Fork 34.5k
bpo-35813: Tests and docs for shared_memory #11816
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
44 commits
Select commit
Hold shift + click to select a range
720a8ea
Added tests for shared_memory submodule.
applio 29a7f80
Added tests for ShareableList.
applio c56e29c
Fix bug in allocationn size during creation of empty ShareableList il…
applio c36de70
Initial set of docs for shared_memory module.
applio 3c89c7c
Added docs for ShareableList, added doctree entry for shared_memory s…
applio 5f4ba8f
Added examples to SharedMemoryManager docs, for ease of documentation…
applio f9aaa11
Wording tweaks to docs.
applio 2377cfd
Fix test failures on Windows.
applio 6bfa560
Added tests around SharedMemoryManager.
applio eaf7888
Documentation tweaks.
applio e166ed9
Fix inappropriate test on Windows.
applio 0f18511
Further documentation tweaks.
applio a097dbb
Fix bare exception.
applio 7c65017
Removed __copyright__.
applio da7731d
Fixed typo in doc, removed comment.
applio 242a5e9
Merge remote-tracking branch 'upstream/master' into enh-tests-shmem
applio 7bdfbbb
Updated SharedMemoryManager preliminary tests to reflect change of no…
applio eec4bb1
Added Sphinx doctest run controls.
applio 1076567
CloseHandle should be in a finally block in case MapViewOfFile fails.
applio 0be0531
Missed opportunity to use with statement.
applio 1e5341e
Switch to self.addCleanup to spare long try/finally blocks and save o…
applio a5800a9
Simplify the posixshmem extension module.
nascheme 34f1e9a
Added to doc around size parameter of SharedMemory.
applio 9846290
Changed PosixSharedMemory.size to use os.fstat.
applio 1f9bbf2
Change SharedMemory.buf to a read-only property as well as NamedShare…
applio 69dd8a9
Marked as provisional per PEP411 in docstring.
applio 8cf9ba3
Merge branch 'enh-tests-neilsimplify-shmem' into enh-tests-shmem
applio 594140a
Changed SharedMemoryTracker to be private.
applio 395709b
Removed registered Proxy Objects from SharedMemoryManager.
applio aa4a887
Removed shareable_wrap().
applio 885592b
Removed shareable_wrap() and dangling references to it.
applio 9001b76
Merge remote and local branches regarding elimination of
applio 5848ec4
For consistency added __reduce__ to key classes.
applio 6ff8eed
Fix for potential race condition on Windows for O_CREX.
applio 06620e2
Remove unused imports.
applio 868b83d
Update access to kernel32 on Windows per feedback from eryksun.
applio 9d83b06
Moved kernel32 calls to _winapi.
applio 715ded9
Removed ShareableList.copy as redundant.
applio 6878533
Changes to _winapi use from eryksun feedback.
applio 0d3d06f
Adopt simpler SharedMemory API, collapsing PosixSharedMemory and Wind…
applio 05e26dd
Fix missing docstring on class, add test for ignoring size when attac…
applio 7a3c7e5
Moved SharedMemoryManager to managers module, tweak to fragile test.
applio caf0a5d
Tweak to exception in OpenFileMapping suggested by eryksun.
applio 12c097d
Mark a few dangling bits as private as suggested by Giampaolo.
applio File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Updated SharedMemoryManager preliminary tests to reflect change of no…
…t supporting all registered functions on SyncManager.
- Loading branch information
commit 7bdfbbb71908aa7af236058453a0618e64ff1580
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why you need to move these 2 methods from the previous tests. The logic appears split between the mixing class and this one for no apparent reason (but I may be missing it). If you need a different logic for certain
TestSharedMemoryManagerTypesmethods/tests you can simply override them (e.g. if you want to skip them).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both
QueueandJoinableQueueare implemented as distributed shared memory (aqueue.Queueobject existing in one process and anAutoProxyobject existing in the local process). For the same reasons mentioned in my b.p.o. comment, I believe it is much clearer to not include these onSharedMemoryManagerby default and to instead document how they may be added on-demand with a single line of code (by callingregister()).There is also the potential for a POSIX shared memory message queue (not currently implemented in shared_memory.py). This is also something worth considering for the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: if we expose them they are not subject to the speedup, correct? Would it be the same as using them via the
SyncManager?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, exposing
QueueorJoinableQueuehere would not make them faster; they would be the same as if using them viaSyncManager.We could simply encourage users to employ a
SyncManageranytime they want to use aQueueorLockbut this forces the creation of two managers with a server behind each and therefore two processes instead of only one. I think it will be easier to explain, "create one manager to oversee work and provide cleanup at the end."Although my use of
empty()in this example is admittedly fragile, this example shows augmentingSyncManagerwithQueueis really only one line of code:Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also think we should get rid of Queue, Lock, etc. Just for confirmation:
ShareableListandAsyncManager.Lockcan be safely mixed/used together, correct?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed -- I have now gotten rid of Queue, Lock, etc. in commit 395709b.
Yes, confirmed:
ShareableListandSyncManager.Lockcan be safely mixed/used together.I was sure you meant
SyncManagerwhen your wroteAsyncManagerbut I still went searching just to make sure there was not someAsyncManagerhiding insideasynciosomewhere that I was not thinking of. =)