Shared mem #28
Merged
Shared mem #28
Conversation
This file contains 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
MattJaniszewski
force-pushed
the
shared-mem
branch
from
e9cd985
to
9136f31
Compare
|
👍 |
| * | ||
| * @return array An array of messages | ||
| */ | ||
| public function receiveMessages() |
There was a problem hiding this comment.
In what cases will the result value contain more then one message?
Shouldn't this method be rather called 'receive' as the opposite of 'send'?
There was a problem hiding this comment.
Thanks Oleg. I took a look again at this code, and realized that this method wasn't actually working as advertised. I think that I fixed the logic now, so take another look when you can.
MattJaniszewski
force-pushed
the
shared-mem
branch
2 times, most recently
from
6ecc684
to
507011c
Compare
|
Would you mind removing the PSR-2 commit (or moving it to another pull request) so that this pull request is limited to the shared memory change? |
MattJaniszewski
force-pushed
the
shared-mem
branch
2 times, most recently
from
2dc4bc2
to
f78c8f9
Compare
…the PHP shared memory subsystem to establish communication between parent and child processes after they are forked. Consolidated the `receiveOne()` and `receiveMany()` methods to one method: `receiveMessages()`. The entire array of messages being sent will be serialized, rather than separating individual serialized message objects by newlines, and writing those to shared memory. This makes unserializing the messages inside `receiveMessages()` a lot easier, and doesn't require special cases to determine differences between object separators and "real" newlines. Added unit tests which cover the cases which made this particular fix necessary. Removed pass-by-reference operators for the arguments passed to `register_shutdown_function()`.
MattJaniszewski
force-pushed
the
shared-mem
branch
from
f78c8f9
to
fd77281
Compare
|
I rebased this branch, and removed all the un-related modifications. I created separate branches for them, and I'll create the pull requests later (so they will end up being fast-forward merges). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I ran into a limit when the FIFO pipe would get full (> ~62k of data), and cause the child processes to block until the pipe was cleared (e.g., by running the
catcommand in the appropriate FIFO handle on the file system). The use of theshmop_functions seemed like a good alternative.In the PR, I added a unit test which re-creates the problem that kept the original version from working on a typical Linux system.
One thing I will note is that while I did all the due-diligence I could with the
shmop_functions, I had to end up using@shmop_openwith the error suppressor operator twice, to test for the presence for an existing memory segment. It's not ideal, but seemed like the only way.A decent way to test that there are no left-over open memory segments is to run
ipcs -mon the Linux system. You can then pass any offendingshmidvalue over to theipcrmbinary. My testing as of yet indicated no issues of this kind, but it's theoretically possible it can happen in some edge-cases.