Nested IF Improvement by kinncj · Pull Request #53 · php/php-src

kinncj · 2012-04-11T20:01:54Z

The code is correct. Just removing the nested IF, because there is no need

… need.

johannes · 2012-04-11T22:52:23Z

I consider the long line harder to read. The compiler will most likely create the same code ...

kinncj · 2012-04-11T23:08:38Z

I agree, but looking at the performance, when running the first time, if it does not have nested, the performance is higher than the nested. The second time we no longer feel the difference because it is already in memory... and more readable code like Dustin Boswell says

johannes · 2012-04-11T23:15:06Z

Did you measure the performance? As said the compiler will produce the same code and even if it wouldn't this would only matter if the item can't be added, which should never happen. So let's keep the better readable code.

kinncj · 2012-04-11T23:38:00Z

Ok, i'll accept this, but i agree with Dustin Boswell and Trevor Foucher about Nesting

…rack the actual socket state at OS level - Modify network_async_set_socket_blocking() to check current state before making system calls, avoiding redundant fcntl() operations - Update function calls in xp_socket.c to pass socket structure for state tracking - Initialize the flag to false (blocking mode) in socket factory

Complete socket polling optimization by adding async_await_socket() to PHP_STREAM_OPTION_CHECK_LIVENESS in php_sockop_set_option(). Now all socket I/O operations (read, write, liveness check) use event handle reuse instead of creating new polling structures on each call.

- Add PHP_STREAM_OPTION_ASYNC_EVENT_HANDLE constant - Implement support in socket streams (xp_socket.c) - Replace unsafe type checks in process_stream_array() with safe Stream API - Socket streams get event handle reuse optimization - Fallback to ZEND_ASYNC_NEW_POLL_EVENT for non-socket streams Prevents crashes from unsafe type casting and improves code safety.

Data structures refactored, code optimized.

…ve keys - Replace inefficient temporary array approach with direct callback-based population - Add key preservation to maintain original array indices per PHP stream_select() spec - Refactor async_stream_callback_t to store references to result arrays - Implement real-time stream insertion during event resolution - Extract duplicate hash insertion logic into inline helper function - Use zend_hash_add() instead of zend_hash_update() for cleared arrays This eliminates 3 temporary HashTables and associated copy operations while ensuring original array keys are preserved as documented.

…nt handling This commit addresses architectural inconsistencies in async socket event management by unifying the approach used across the codebase. Changes: - Modified network_async_await_stream_socket() to accept php_stream* instead of php_netstream_data_t*, enabling access to the standard Stream API - Replaced direct ZEND_ASYNC_NEW_SOCKET_EVENT() calls with unified php_stream_set_option(PHP_STREAM_OPTION_ASYNC_EVENT_HANDLE) approach - Updated all call sites in xp_socket.c to pass stream instead of sock data - Ensured consistent event handle creation/retrieval across all async operations

…occurs, php_network_accept_incoming outputs its message as a warning.

…ad of php_poll2_async This commit replaces the inefficient php_poll2_async calls with the modern network_async_await_stream_socket system to prevent EventLoop duplication and improve performance. Changes: * Add network_async_accept_incoming() - async version of php_network_accept_incoming * Add network_async_connect_socket() - async version of php_network_connect_socket * Update xp_socket.c to use conditional logic: - ZEND_ASYNC_IS_ACTIVE: use new efficient async functions - Otherwise: use original sync functions * Add function declarations to network_async.h

- Replace php_poll2_async with network_async_accept_incoming() to prevent EventLoop conflicts - Add network_async_connect_socket() for consistent async connection handling - Fix SSL socket EventLoop duplication in accept and close operations - Add proper poll_event cleanup in SSL sockets to prevent memory leaks - Create SSL async tests for timeout, client-server, and concurrent operations + test 025-ssl_stream_socket_accept_timeout.phpt

- Replace corrupted base64 certificate with valid RSA 2048-bit cert - Use temporary files instead of data:// URIs for Windows OpenSSL compatibility - Add explicit TLS crypto methods for server and client contexts - Fix client SSL connection by adding ssl:// prefix to server address - Add proper cleanup of temporary certificate files

Replace `clisock >= 0` comparisons with `ZEND_VALID_SOCKET(clisock)` to properly handle Windows SOCKET type (unsigned int) where INVALID_SOCKET = ~0. Files modified: - main/streams/xp_socket.c:973 - ext/openssl/xp_ssl.c:2266

…before waiting - Try accept() immediately instead of waiting for READ event first - Only wait in event loop if accept() returns EAGAIN/EWOULDBLOCK - Eliminates unnecessary syscalls when connections are already ready - Refactor code structure with centralized error handling via goto - Use EXPECTED/UNEXPECTED macros and const variables for better optimization

…nsure non-blocking mode - Try accept() immediately instead of waiting for READ event first - Only wait in event loop if accept() returns EAGAIN/EWOULDBLOCK - Ensure socket is in non-blocking mode before accept() calls - Eliminates unnecessary syscalls when connections are already ready - Refactor with centralized error handling and better code structure

…mentations Apply deferred socket closure mechanism to prevent LibUV race conditions in both xp_socket.c and xp_ssl.c modules. Previously, closesocket() was called before poll_event cleanup, causing potential race conditions where LibUV operations could access already-closed socket descriptors. Changes: - xp_socket.c: Fix php_sockop_close() to transfer socket ownership to LibUV before disposal - xp_ssl.c: Apply same fix to php_openssl_sockop_close() with SSL-specific cleanup preserved - Move Windows async logic (shutdown/polling) before socket ownership transfer in both modules - When poll_event exists, transfer socket via poll_event->socket and ZEND_ASYNC_EVENT_SET_CLOSE_FD flag - Call poll_event dispose immediately after flag setting for proper cleanup order - libuv_reactor.c: Add libuv_close_poll_handle_cb() with cross-platform fd/socket closure - zend_async_API.h: Define ZEND_ASYNC_EVENT_F_CLOSE_FD flag for conditional descriptor closure This ensures LibUV completes all pending operations before socket closure across both socket implementations.

Implements a new poll proxy structure that allows multiple consumers to share a single zend_async_poll_event_t, reducing file descriptor and libuv handle usage. Key features: - zend_async_poll_proxy_t holds reference to shared poll event - Event aggregation: proxy events are OR'ed into base event - Reference counting for automatic cleanup - Standard event interface (start/stop/dispose/callbacks) Changes: - Add zend_async_poll_proxy_t structure and typedef - Add libuv_new_poll_proxy_event() creation function - Implement proxy start/stop/dispose methods with event aggregation - Add ZEND_ASYNC_NEW_POLL_PROXY_EVENT() macros - Update zend_async_reactor_register() signature and calls - Add zend_async_new_poll_proxy_event_fn function pointer Usage: zend_async_poll_event_t *base = ZEND_ASYNC_NEW_POLL_EVENT(fd, 0, events); zend_async_poll_proxy_t *proxy = ZEND_ASYNC_NEW_POLL_PROXY_EVENT(base, specific_events); This enables efficient FD sharing while maintaining clean abstraction.

Implement proxy events that allow multiple consumers to share a single zend_async_poll_event_t, reducing FD and libuv handle usage. Fix EV integration by returning separate proxy events instead of shared base events, preventing premature event cleanup when coroutines exit.

Add cleanup for read_event/write_event proxy fields in SSL socket disposal, ensuring proper resource management for both sync and async cleanup paths. SSL sockets already inherit proxy event functionality through structure embedding and operation delegation.

…xy_event

The optimization for reusing cached async events was not working for PHP_POLLREADABLE because it gets converted to composite flags (ASYNC_READABLE | ASYNC_DISCONNECT = 5) instead of pure ASYNC_READABLE (1). Changes: - Add READ_EVENT_MASK to define events covered by read_event cache - Change exact equality checks to subset matching for read events - Keep write_event logic unchanged (exact ASYNC_WRITABLE matching) This allows read_event to be reused for any combination of readable events (ASYNC_READABLE, ASYNC_DISCONNECT, or both), avoiding unnecessary proxy creation for common polling scenarios.

…ctions Add missing asynchronous connection support for TCP/UDP sockets in php_tcp_sockop_connect. Previously only Unix domain sockets used async when ZEND_ASYNC_IS_ACTIVE, while TCP/UDP always used synchronous php_network_connect_socket_to_host. Changes: - Add php_stream parameter to php_network_connect_socket_to_host() - Use network_async_connect_socket() when stream provided and ZEND_ASYNC_IS_ACTIVE - Update FTP extension call (pass NULL to remain synchronous) - Update socket streams call (pass stream to enable async)

Introduce php_network_connect_socket_to_host_ex() with php_netstream_data_t parameter to enable async connections while preserving ABI compatibility. The original function becomes a wrapper calling _ex version with NULL. Key changes: - Add php_netstream_get_async_event() for direct netstream async event management - Create _ex version of php_network_connect_socket_to_host with netdata parameter - Update network_async_connect_socket to use direct netstream function instead of php_stream_set_option - Socket streams now use _ex version to get async support when ZEND_ASYNC_IS_ACTIVE - FTP remains synchronous using original function - Eliminate circular dependency: network → stream → network This ensures TCP/UDP sockets work asynchronously in async context, matching existing Unix socket behavior, while maintaining full backward compatibility.

…a_t directly Replace php_stream* parameters with php_netstream_data_t* in network_async module functions to eliminate pointer indirection and improve performance. Changes: - network_async_await_stream_socket(): php_stream* → php_netstream_data_t* - network_async_accept_incoming(): php_stream* → php_netstream_data_t* - network_async_connect_socket(): php_stream* → php_netstream_data_t* Files modified: - main/network_async.h: Updated function declarations - main/network_async.c: Updated function implementations - main/streams/xp_socket.c: Updated 4 function calls - ext/openssl/xp_ssl.c: Updated 2 function calls (using &sslsock->s) - main/network.c: Updated 1 function call, simplified temp stream creation

Extend network_async functionality to UDP send/recv operations: * sock_sendto() now supports async writes with poll-first approach * sock_recvfrom() now supports async reads with poll-first approach * Consistent with existing TCP async implementation * Uses network_async_await_stream_socket() for efficient polling * Maintains backward compatibility for non-async contexts This enables UDP sockets to work seamlessly in async/coroutine environments without blocking the event loop.

* Add sock_async_poll() helper function for shared async setup/polling logic * Enable sock_sendto() and sock_recvfrom() to work in async contexts * Use poll-first approach with network_async_await_stream_socket() * Add UNEXPECTED() hints for better branch prediction optimization * Eliminate code duplication between send/recv async implementations * Maintain full backward compatibility with synchronous operation

Changed network_async_set_socket_blocking from void to bool return type to eliminate redundant EG(exception) checks. Function now returns false on error instead of requiring separate exception checking. Updated 3 call sites: - php_sockop_write() in xp_socket.c - sock_async_poll() in xp_socket.c - network_async_accept_incoming() in network_async.c This reduces code duplication and improves performance by combining function call and error check into single conditional.

- Reduce UDP timeout from 2s to 0.2s in test 030 - Reduce TCP timeout from 1s to 0.2s in test 031 - Remove timing measurements and elapsed time checks - Simplify timeout validation to only check timed_out flag - Reduce client delay from 2s to 0.5s in TCP tes

This commit fixes a critical memory management issue in the async event callback system where callbacks were created with ref_count=1 but add methods didn't increment the counter, leading to premature deallocation. Changes: - Initialize all callbacks with ref_count=0 instead of 1 - Add ref_count increment in callback registration methods - Remove incorrect ref_count validation in zend_async_resume_when Modified files: * Zend/zend_async_API.c: Fix zend_async_coroutine_callback_new and waker_trigger_add_callback * Zend/zend_async_API.h: Fix zend_async_callbacks_push * main/network_async.c: Fix 5 direct callback initializations for poll_callback_t, select_callback_t, async_stream_callback_t, and dns_callback_t structures This ensures proper lifetime management for all async callback types including socket, DNS, stream, and scope callbacks.

…description for the changes: Optimize exception handling in ext/async with fast save/restore operations This commit replaces standard exception save/restore patterns with optimized fast variants throughout the ext/async extension to improve performance during coroutine context switching and async operations. Changes include: * scheduler.c: Optimize 7+ exception handling points including: - TRY_HANDLE_SUSPEND_EXCEPTION macro implementation - cancel_queued_coroutines() function - async_scheduler_coroutine_suspend() function - async_scheduler_dtor() cleanup * coroutine.c: Optimize save/restore pairs in: - async_coroutine_finalize() - finally_handlers_iterator_handler() * scope.c: Optimize scope completion callback exception handling * zend_common.c: Optimize zend_exception_merge() function * exceptions.c: Optimize async_extract_exception() function The optimization uses direct pointer manipulation instead of repeated EG() macro access, following the pattern: zend_object **exception_ptr = &EG(exception); zend_object **prev_exception_ptr = &EG(prev_exception); zend_exception_save_fast(exception_ptr, prev_exception_ptr); // ... code execution ... zend_exception_restore_fast(exception_ptr, prev_exception_ptr); This reduces overhead in exception state management during async operations while maintaining identical exception handling semantics.

…ase.ref_count >= 0 && "Callback ref_count must be non-negative.");

Move variable declaration to function start to prevent use of uninitialized variable in error handling path when exceptions occur early in the function.

The code is correct. Just removing the nested if, because there is no…

e0151fb

… need.

johannes closed this Apr 11, 2012

dunglas mentioned this pull request Sep 17, 2023

Segmentation fault when building Symfony cache on Alpine #12234

Closed

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 7, 2025

php#53: Refactoring process_stream_array

e9e4c76

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 7, 2025

php#53: Refactoring process_stream_array2

b69b338

Data structures refactored, code optimized.

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 7, 2025

php#53: Refacroting network_async.c

1cd40d5

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 8, 2025

php#53: * fixes to STREAM test behavior

09dbef1

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 8, 2025

php#53: + Add new tests for stream_select()

02cecd9

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 9, 2025

php#53: + 024-stream_select_remote_disconnect.phpt

121e960

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 9, 2025

php#53: A more proper error handling has been added. If an exception …

f593704

…occurs, php_network_accept_incoming outputs its message as a warning.

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 10, 2025

php#53: * fix warning: this statement may fall through

477d9db

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 12, 2025

php#53: return code for UNIX-style

af6b7cd

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 15, 2025

php#53: + ZEND_ASYNC_WAKER_CLEAN_EVENTS

e92dacc

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 15, 2025

php#53: Fix 010-stream_select_async and issue with poll event => pro…

322e26a

…xy_event

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 15, 2025

php#53: Fix stream tests

aa984e7

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 17, 2025

php#53: * fix php_network_connect_socket_to_host_ex

5e64d17

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 17, 2025

php#53: * fix INVALID_SOCKET issue

27d31cd

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 17, 2025

php#53: * fixes for php_netstream_get_async_event

6aa47a4

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 17, 2025

php#53: * fixes for 004-stream_socket_client_server.php

7dff544

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 19, 2025

php#53: fix sock_async_poll

50cf37c

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 20, 2025

php#53: * fix deadlock tests and fix zend_async_event_callback_new

abda127

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 21, 2025

php#53: * fix php_accept_connect for accept4

a9d35b4

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 21, 2025

php#53: * fix php_accept_connect for accept4

e69af46

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 21, 2025

php#53: * fix php_accept_connect for accept4

50313a1

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 21, 2025

php#53: * fix warning with non negative ZEND_ASSERT(event_callback->b…

5cf7d22

…ase.ref_count >= 0 && "Callback ref_count must be non-negative.");

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 22, 2025

php#53: * Fix async_wait_process logic

ca9cc42

Navigation Menu

Repository navigation

Nested IF Improvement - #53

Nested IF Improvement#53
kinncj wants to merge 1 commit into
php:masterfrom
kinncj:patch-1

kinncj commented Apr 11, 2012

Uh oh!

johannes commented Apr 11, 2012

Uh oh!

kinncj commented Apr 11, 2012

Uh oh!

johannes commented Apr 11, 2012

Uh oh!

kinncj commented Apr 11, 2012

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

kinncj commented Apr 11, 2012

Uh oh!

johannes commented Apr 11, 2012

Uh oh!

kinncj commented Apr 11, 2012

Uh oh!

johannes commented Apr 11, 2012

Uh oh!

kinncj commented Apr 11, 2012

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants