[WIP] Immediate writes to underlying stream until buffer is filled#54
Uh oh!
There was an error while loading. Please reload this page.
Conversation
clue commented Aug 21, 2016
clue changed the title clue commented Aug 22, 2016
Rebased now that #52 is in. Ready for review |
WyriHaximus commented Aug 22, 2016
LGTM |
davidwdan commented Oct 19, 2016
Can we get this merged? It bit me today and wasted many hours. |
WyriHaximus commented Oct 19, 2016
Agreed @davidwdan lets get this in. @clue what is holding us back from merging this and related issues? |
| $buffer = newBuffer($stream, $loop); | ||
| $buffer->softLimit = 4; | ||
| $buffer->on('error', $this->expectCallableNever()); | ||
| $buffer->on('drain', $this->expectCallableOnce()); |
There was a problem hiding this comment.
Does it really make sense to emit a drain event here?
After reconsidering this, we should probably not trigger this event if the data can be flushed immediately. Even though the data may exceed the buffer size, this is an atomic step that can not be observed from the outside.
Instead, we should probably only trigger this event if the buffer returned false previously and it is now ready again.
clue commented Oct 19, 2016
Absolutely! 👍
Huh, care to elaborate? |
clue commented Nov 13, 2016
Rebased now that #55 is in. I've moved this PR to the next milestone because it requires an update to not emit a |
clue commented Dec 19, 2016
Removing the milestone for now until the |
clue changed the title clue dismissed WyriHaximus’s stale reviewTurns out we need to reconsider the drain event first
clue commented May 3, 2017
The main focus is currently on getting the v1.0.0 out. As such, there are currently no immediate plans to build this from my end, so I'm closing this for now 👍 |
mbonneau commented Jan 16, 2018
@clue - Any chance of getting this going again? |
clue commented Jan 16, 2018
@mbonneau I'm currently busy working on the other components and this feature does not seem to have a high priority right now, but in case anybody feels like picking this up, I'm happy to help |
4 participants
With this change applied, the
Bufferwill immediately write to the underlying stream instead of waiting for the loop to say it it writable. This is safe because the stream is non-blocking anyway and the Buffer class will return to normal buffer behavior once the data can not be flushed immediately.Running the examples/benchmark-throughput.php script, this change alone increases performance from ~100 MiB/s to ~160 MiB/s on my (rather mediocre) test system, using PHP 7.0.8.
Combined with #53, it increases performance from ~100 MiB/s to ~1900 MiB/s.
Marking this as WIP because this needs some additional tests once #52 is in.Edit: done.