Skip to content

perf(s3): share DNS, TLS session and connection cache across requests#169

Closed
loks0n wants to merge 1 commit into
mainfrom
perf-s3-curl-share
Closed

perf(s3): share DNS, TLS session and connection cache across requests#169
loks0n wants to merge 1 commit into
mainfrom
perf-s3-curl-share

Conversation

@loks0n

@loks0n loks0n commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

What

Attach a process-wide curl share handle (CURL_LOCK_DATA_DNS + CURL_LOCK_DATA_SSL_SESSION + CURL_LOCK_DATA_CONNECT) to every S3 request. Easy handles remain per-call (coroutine-safe); only the DNS cache, TLS session cache and connection pool are shared.

Why

S3::call creates and destroys a curl handle per operation, so every PUT/HEAD/GET pays DNS + TCP + TLS setup. Profiling Appwrite Cloud's builds worker during a backlog showed curl_exec in S3::write/S3::getInfo as the top CPU/wall sink; cluster DNS degraded to 1.7s–16s per lookup under the concurrent-clone burst, multiplying per-op cost.

Measured (in-cluster, DO Spaces fra1, Swoole SWOOLE_HOOK_ALL)

request total dns connect tls
cold 80ms 33ms 37ms 69ms
warm (shared) 6–11ms 0 0 0

Verified against PHP 8.5.7 / curl 8.19.0 under Swoole's native curl hook.

🤖 Generated with Claude Code

Every S3 call previously created and destroyed its own curl handle,
paying a fresh DNS lookup, TCP connect and full TLS handshake per
operation. Under load (e.g. build storms hammering cluster DNS) this
dominated request time: measured 80ms cold vs 6ms warm per request
in-cluster, and seconds per call when DNS is degraded.

A process-wide curl share handle keeps the DNS cache, TLS session
cache and connection pool shared across the per-call easy handles,
which stay per-call and therefore coroutine-safe. Verified working
under Swoole's native curl hook (SWOOLE_HOOK_ALL).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Thanks for contributing! This repository is a read-only mirror; development for this library happens in packages/storage in the utopia-php monorepo. Please open this pull request there instead.

@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a shared curl cache path for S3 requests. The main changes are:

  • A static curl share handle on the S3 device.
  • Lazy initialization of shared DNS, TLS session, and connection caches.
  • Attachment of the share handle to each per-request curl handle.

Confidence Score: 4/5

The shared S3 connection cache can break overlapping Swoole requests.

  • Concurrent S3 calls can enter curl_exec() with separate easy handles that share one connection cache.
  • The curl share initialization path can crash if initialization fails.
  • PHP and curl version requirements cover the new type and constants.

src/Storage/Device/S3.php

Important Files Changed

Filename Overview
src/Storage/Device/S3.php Adds a process-wide curl share handle for S3 requests, with concurrency and initialization risks in the changed curl setup path.

Fix All in Claude Code Fix All in Codex

Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/Storage/Device/S3.php:740
**Shared Connection Cache Races**

When Swoole runs overlapping S3 operations, each coroutine creates its own easy handle but now points it at the same process-wide connection cache. libcurl does not support sharing `CURL_LOCK_DATA_CONNECT` across concurrent transfers, so concurrent `curl_exec()` calls can intermittently fail or reuse connection state incorrectly under the workload this change targets.

### Issue 2 of 2
src/Storage/Device/S3.php:737
**Failed Share Init Crashes**

If `curl_share_init()` returns `false`, this assignment writes a boolean into a `?\CurlShareHandle` static property before the existing S3 error handling can run. That turns a curl setup failure into a PHP `TypeError` for the request instead of a controlled storage error or a fallback to an unshared handle.

Reviews (1): Last reviewed commit: "perf(s3): share DNS, TLS session and con..." | Re-trigger Greptile

Comment thread src/Storage/Device/S3.php
self::$curlShare = curl_share_init();
curl_share_setopt(self::$curlShare, CURLSHOPT_SHARE, CURL_LOCK_DATA_DNS);
curl_share_setopt(self::$curlShare, CURLSHOPT_SHARE, CURL_LOCK_DATA_SSL_SESSION);
curl_share_setopt(self::$curlShare, CURLSHOPT_SHARE, CURL_LOCK_DATA_CONNECT);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Shared Connection Cache Races

When Swoole runs overlapping S3 operations, each coroutine creates its own easy handle but now points it at the same process-wide connection cache. libcurl does not support sharing CURL_LOCK_DATA_CONNECT across concurrent transfers, so concurrent curl_exec() calls can intermittently fail or reuse connection state incorrectly under the workload this change targets.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Storage/Device/S3.php
Line: 740

Comment:
**Shared Connection Cache Races**

When Swoole runs overlapping S3 operations, each coroutine creates its own easy handle but now points it at the same process-wide connection cache. libcurl does not support sharing `CURL_LOCK_DATA_CONNECT` across concurrent transfers, so concurrent `curl_exec()` calls can intermittently fail or reuse connection state incorrectly under the workload this change targets.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Codex

Comment thread src/Storage/Device/S3.php

// Basic setup
if (self::$curlShare === null) {
self::$curlShare = curl_share_init();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Failed Share Init Crashes

If curl_share_init() returns false, this assignment writes a boolean into a ?\CurlShareHandle static property before the existing S3 error handling can run. That turns a curl setup failure into a PHP TypeError for the request instead of a controlled storage error or a fallback to an unshared handle.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Storage/Device/S3.php
Line: 737

Comment:
**Failed Share Init Crashes**

If `curl_share_init()` returns `false`, this assignment writes a boolean into a `?\CurlShareHandle` static property before the existing S3 error handling can run. That turns a curl setup failure into a PHP `TypeError` for the request instead of a controlled storage error or a fallback to an unshared handle.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Codex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant