Skip to content

Conversation

clue
Copy link
Member

@clue clue commented Dec 6, 2021

This changeset reduces the socket read chunk size for queries over TCP/IP to a reasonable value of 64 KiB. PHP's fread() uses an underlying read() call with the given chunk size (defaults to 8 KiB) instead of the size limit given to the fread() call. In an attempt to address a race condition on Mac, I've accidentally raised this default chunk size to 2 GiB via #172/#177. This means that any socket read would temporarily consume 2 GiB of memory, even if we only read a few bytes of data. Ouch.

With these changes applied, the test suite now reports a memory usage of ~84 MB instead of 8 GB(!). I've only stumbled upon this recently while addressing PHP 8.1 compatibility (#188) and trying to execute this with PHP's default memory_limit of just 128 MiB. The good news is this didn't usually happen during normal operation, as the TCP/IP transport will only be used as a fallback if the UDP query returns a truncated message.

@clue clue added this to the v1.9.0 milestone Dec 6, 2021
@clue clue requested a review from WyriHaximus December 6, 2021 18:50
@WyriHaximus WyriHaximus merged commit e5e152a into reactphp:master Dec 6, 2021
@clue clue deleted the memory branch December 6, 2021 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants