streams/xp_socket: eliminate poll() when MSG_DONTWAIT is available

[MaxKellermann]

If there is a zero timeout and MSG_DONTWAIT is available (or the
socket is non-blocking), the poll() call is not necessary, and we can
just call recv() right away.

Before this change:

poll([{fd=4, events=POLLIN|POLLPRI|POLLERR|POLLHUP}], 1, 0) = 0 (Timeout)
poll([{fd=4, events=POLLIN|POLLERR|POLLHUP}], 1, 60000) = 1 ([{fd=4, revents=POLLIN}])
recvfrom(4, "HTTP/1.1 301 Moved Permanently\r\n"..., 8192, MSG_DONTWAIT, NULL, NULL) = 348
poll([{fd=4, events=POLLIN|POLLPRI|POLLERR|POLLHUP}], 1, 0) = 1 ([{fd=4, revents=POLLIN}])
recvfrom(4, "", 1, MSG_PEEK, NULL, NULL) = 0

After this change:

recvfrom(4, 0x7ffe0cc719a0, 1, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
poll([{fd=4, events=POLLIN|POLLERR|POLLHUP}], 1, 60000) = 1 ([{fd=4, revents=POLLIN}])
recvfrom(4, "HTTP/1.1 301 Moved Permanently\r\n"..., 8192, MSG_DONTWAIT, NULL, NULL) = 348
recvfrom(4, "", 1, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 0

The first poll() is replaced by recvfrom(), and the third poll() is
omitted completely.

[Splitted from https://github.com//pull/8032]

[MaxKellermann]

CI failures unrelated to this PR.

[cmb69]

Please rebase onto current php-src:master and force-push, to re-trigger CI.

[MaxKellermann]

Yay, "All checks have passed".

[MaxKellermann]

I rebased; AppVeyor failure unrelated to this PR.

[twose]

LGTM

[bukka]

It makes sense to me. Do we actually have any test case covering this path?

[twose]

@MaxKellermann The same optimization can also be applied to php_openssl_sockop_set_option.

It makes sense to me. Do we actually have any test case covering this path?

@bukka I believe that many socket-stream-related tests have covered this path, such as the most straightforward case for me: echo file_get_contents('http://www.baidu.com');, it also covers this path.

[devnexen]

@MaxKellermann do you still want to proceed with your open PRs ? still few weeks left before the feature freeze :-)

[MaxKellermann]

@devnexen yes, of course.
I didn't see this was tagged "Waiting on Author" by @ramsey, but what are you waiting for?
(@twose suggested another optimization, but such an idea for an additional optimization shouldn't delay merging this PR, should it?)

[devnexen]

would it require a lot of changes ? otherwise yes it s mergeable as is.

[MaxKellermann]

I've added the same optimization to ext/openssl as suggested by @twose, but only if !ssl_active, because I don't know for sure if the OpenSSL APIs will do non-blocking recv() properly. There may be a way, but that's an optimization for another day.

[MaxKellermann]

MACOS_DEBUG_NTS build failure unrelated to this PR.

[cmb69]

The Windows build fails, because MSG_DONTWAIT is not defined there. Not sure how to solve that best; maybe temporarily define it to 0; or maybe introduce platform specifc code (#ifdef PHP_WIN32).

[MaxKellermann]

The Windows build fails, because MSG_DONTWAIT is not defined there. Not sure how to solve that best; maybe temporarily define it to 0; or maybe introduce platform specifc code (#ifdef PHP_WIN32).

There's a fallback definition in xp_socket.c but not in xp_ssl.c:

php-src/main/streams/xp_socket.c

Lines 30 to 32 in 790be97

	#ifndef MSG_DONTWAIT
	# define MSG_DONTWAIT 0
	#endif

Will add it.

[MaxKellermann]

Interestingly, xp_socket.c has a fallback definition for both MSG_DONTWAIT and MSG_PEEK and uses both; on the other hand, xp_ssl.c does already use MSG_PEEK but has no fallback definition. That MSG_PEEK fallback appears to be not necessary at all.

[bukka]

@MaxKellermann I had a proper look at this. All looks reasonable to me. I think there's a possible optimization in xp_ssl that I just did at #8829 . Please take a look and let me know in case there's a specific reason why you haven't done it in this PR.