53 optimize descriptor registration in event loop #60
Conversation
This file contains hidden or 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
…lect
Replace per-connection coroutines with centralized event loop using stream_select() to monitor all active sockets simultaneously. Main loop now handles both new connections
and existing client requests through single event monitoring, eliminating "Bad File handler" errors by proper socket lifecycle management.
Key changes:
- Single stream_select() monitors server + all active client sockets
- handleSingleRequest() processes one request per invocation
- Automatic cleanup of closed connections from activeConnections array
- Proper Keep-Alive support through socket reuse
- 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
- Extract valid certificate from test 026 to ssl_test_cert.pem and ssl_test_key.pem
- Replace malformed certificate in test 025 with valid shared certificate
- Simplify test 026 by removing temp file creation, use external cert files
- Fix SKIPIF bug in test 027 and update to use shared certificate files
- Improve code reuse, maintainability, and test reliability
Replace broken stream_select logic with simple coroutine-per-connection.
Each socket now handled by dedicated handleSocket() coroutine for proper keep-alive.
…ore 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
…tations
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.
Corrects the poll proxy implementation to properly handle multiple
proxies sharing the same base poll event through proper event
aggregation and proxy tracking.
Key fixes:
- Add proxies array to async_poll_event_t for tracking active proxies
- Fix libuv_poll_proxy_start(): use bitwise check (events & proxy->events)
to only add missing events, preventing unnecessary libuv restarts
- Fix libuv_poll_proxy_stop(): properly aggregate remaining proxy events
after removing stopped proxy, ensuring correct event mask
- Add proxy array management: async_poll_add/remove_proxy() helpers
- Add memory management: free proxies array in libuv_poll_dispose()
- Fix dispose: call stop() before releasing base event reference
This ensures correct FD sharing where multiple proxies can safely
use overlapping events on the same file descriptor without conflicts.
Before: stop() incorrectly removed events other proxies might need
After: stop() recalculates events from all remaining active proxies
LibUV may return the UV_EBADF code when the remote host closes the connection while the descriptor is still present in the EventLoop. For POLL events, we handle this by ignoring the situation so that the coroutine receives the ASYNC_DISCONNECT flag.
- Make proxy management functions zend_always_inline for performance
- Remove premature memory deallocation in async_poll_remove_proxy()
- Add early exit optimization in async_poll_aggregate_events()
- Simplify async_poll_notify_proxies() - remove alloca() usage
- Fix callback parameters: pass filtered events and exception to proxies
- Only notify proxies with matching events for efficiency
- Add forward declaration to fix compilation order
- Ensure reference counting safety during callback processing
Result: fewer callback invocations, reduced allocations, precise event delivery.
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.
… 53-optimize-descriptor-registration-in-event-loop-exp
Add automatic stopping of waker events for coroutines enqueued after libuv_reactor_execute. This optimization saves file descriptors and improves EV performance by cleaning
up unused poll events immediately after reactor processing.
Features:
- Inline optimized functions with zend_always_inline for zero overhead
- Circular buffer traversal with bitwise operations for power-of-2 optimization
- Branch prediction hints with EXPECTED/UNEXPECTED macros
- Conditional compilation under LIBUV_STOP_WAKER_EVENTS_AFTER_EXECUTE flag
Add time-based throttling to prevent excessive ZEND_ASYNC_REACTOR_EXECUTE calls by checking reactor handles only every 100ms instead of on every tick.
Add automatic cleanup of waker events for coroutines that are queued
after reactor execution. When coroutines become active after ZEND_ASYNC_REACTOR_EXECUTE,
their event lists are immediately cleared to prevent stale event data.
- Add clean_events_for_resumed_coroutines() function to iterate through
newly queued coroutines in circular buffer
- Track head position before/after reactor execution
- Clear waker events using ZEND_ASYNC_WAKER_CLEAN_EVENTS macro
- Optimize with cached mask and item_size values
- Add null checks for coroutine and waker validity
Handle circular buffer reallocation during reactor execution to prevent crashes when cleaning waker events for resumed coroutines. - Track previous_data, previous_count, and previous_head before reactor - Detect reallocation by comparing queue->data pointers - After reallocation: start cleanup from previous_count position - Add assertion to verify tail=0 after reallocation - Maintain original logic for non-reallocation cases
- 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
…cription 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.
…nup stability
The previous clean_events_for_resumed_coroutines algorithm was unstable due to
complex tracking of circular buffer changes during coroutine additions and
buffer reallocations. This commit introduces a dedicated resumed_coroutines
queue that provides a more reliable approach.
Changes:
- Add resumed_coroutines circular buffer to module globals
- Initialize resumed_coroutines queue in PHP_RINIT_FUNCTION
- Track resumed coroutines in async_scheduler_coroutine_enqueue() and async_coroutine_resume()
- Replace complex clean_events_for_resumed_coroutines() with simple process_resumed_coroutines()
- Update scheduler_next_tick() and fiber_entry() to use new queue-based approach
- Add proper cleanup in both engine_shutdown() and async_scheduler_dtor() to prevent memory leaks
Benefits:
- Eliminates race conditions from buffer reallocation tracking
- Simplifies logic by checking queue contents instead of head pointer changes
- Improves code readability and maintainability
- Fixes memory leak of 8192 bytes in circular buffer allocation
…ion-in-event-loop' into 53-optimize-descriptor-registration-in-event-loop
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes descriptor registration in the event loop to improve performance in high-concurrency scenarios. The optimization focuses on reducing overhead in the scheduler and improving exception handling patterns.
- Optimizes event loop descriptor registration with faster exception handling functions
- Improves scheduler performance by reducing reactor tick frequency and adding process-resumed-coroutines functionality
- Adds comprehensive test coverage for stream operations including SSL, UDP, and timeout scenarios
Reviewed Changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| zend_common.c | Updates exception handling to use optimized fast save/restore functions |
| scheduler.c | Major scheduler optimizations including reactor tick throttling and resumed coroutines processing |
| php_async.h | Adds reactor check interval constant and resumed coroutines buffer |
| libuv_reactor.c | Enhances poll event handling with proxy support and improved error handling |
| coroutine.c | Updates coroutine finalization and resume logic with optimized exception handling |
| async.c | Initializes new resumed coroutines buffer and reactor optimization globals |
| tests/stream/* | Adds extensive test coverage for SSL, UDP, TCP operations and edge cases |
| benchmarks/http_server_keepalive.php | Optimizes HTTP server implementation for better performance |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
* Remove --enable-opcache from configure (OPcache is built-in by default)
* Remove -d zend_extension=opcache.so from test commands (legacy syntax)
* Update all workflows: build.yml, build-linux.yml, build-macos.yml, build-alpine.yml, build-freebsd.yml
* Maintains full OPcache and JIT testing functionality
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 45 out of 48 changed files in this pull request and generated 5 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…the application to hang
EdmondDantes
deleted the
53-optimize-descriptor-registration-in-event-loop
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.