Skip to content

Multi-poll backend optimization: configure CURLMOPT_MAXCONNECTS to avoid unnecessary TCP/TLS handshakes - #1034

Open
kingcrimsontianyu wants to merge 3 commits into
rapidsai:mainfrom
kingcrimsontianyu:improve-curl-caching
Open

Multi-poll backend optimization: configure CURLMOPT_MAXCONNECTS to avoid unnecessary TCP/TLS handshakes#1034
kingcrimsontianyu wants to merge 3 commits into
rapidsai:mainfrom
kingcrimsontianyu:improve-curl-caching

Conversation

@kingcrimsontianyu

Copy link
Copy Markdown
Contributor

By default libcurl sets CURLMOPT_MAXCONNECTS to 4 x the number of easy handles attached to a multi handle. This is recomputed on every transition, and a transient dip in concurrency will cause libcurl to evict warm, reusable connections, and cause unnecessary TCP/TLS handshake. In this PR we pin CURLMOPT_MAXCONNECTS to a fixed size to avoid this problem.

Benchmark on cudf-polars TPC-H SF-1K query 5 shows that, on main the TCP connection opened is 618, whereas with this PR, the connection number drops to 496, which is a ~20% saving.

@copy-pr-bot

copy-pr-bot Bot commented Aug 14, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@kingcrimsontianyu kingcrimsontianyu added improvement Improves an existing functionality non-breaking Introduces a non-breaking change c++ Affects the C++ API of KvikIO labels Aug 14, 2026
@kingcrimsontianyu
kingcrimsontianyu marked this pull request as ready for review August 14, 2026 20:49
@kingcrimsontianyu
kingcrimsontianyu requested a review from a team as a code owner August 14, 2026 20:49

@madsbk madsbk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

{
if (!max_concurrent_requests.has_value()) { return std::nullopt; }

// libcurl documents this option as taking a `long`, and the value is internally store as an

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
// libcurl documents this option as taking a `long`, and the value is internally store as an
// libcurl documents this option as taking a `long`, and the value is internally stored as an

std::numeric_limits<std::size_t>::max() / 2,
static_cast<std::size_t>(limit),
static_cast<std::size_t>(limit) / 2 + 1};
for (const auto ceiling : ceilings) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
for (const auto ceiling : ceilings) {
for (auto const ceiling : ceilings) {

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

Labels

c++ Affects the C++ API of KvikIO improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants