Skip to content

Conversation

@ammarfaizi2
Copy link
Contributor

Thank you so much for this amazing library!

Improvement Proposal

The CurlHttpClient currently produces an excessive number of connect() syscalls, as every makeRequest creates a new TCP connection. This inefficiency becomes particularly evident with TgLongPoll, which continuously calls getUpdates in an endless loop, creating a relentless cycle of connect() and close() operations.

A more sensible approach is to reuse HTTP connections by keeping them alive across multiple requests. I suggest modifying the implementation to allow the curl handle to persist until the CurlHttpClient object is destroyed.

For thread safety, we can maintain a std::unordered_map<std::thread::id, CURL*> curlHandles within the CurlHttpClient class, ensuring each thread gets its own handle.

After all, treating connections like disposable napkins doesn't exactly scream efficiency.

Before this change

So many socket connect() and close() calls:

ammarfaizi2@integral2:~/p/ict-bot/build$ strace -e connect ./ict-bot
connect(9, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)
connect(9, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)
connect(9, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)
connect(11, {sa_family=AF_INET6, sin6_port=htons(443), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "2001:67c:4e8:f004::9", &sin6_addr), sin6_scope_id=0}, 28) = -1 EINPROGRESS (Operation now in progress)
connect(9, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)
connect(9, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)
connect(11, {sa_family=AF_INET6, sin6_port=htons(443), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "2001:67c:4e8:f004::9", &sin6_addr), sin6_scope_id=0}, 28) = -1 EINPROGRESS (Operation now in progress)
connect(9, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)
connect(11, {sa_family=AF_INET6, sin6_port=htons(443), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "2001:67c:4e8:f004::9", &sin6_addr), sin6_scope_id=0}, 28) = -1 EINPROGRESS (Operation now in progress)
connect(9, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)
connect(9, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)
connect(9, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)
connect(9, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)
connect(9, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)
connect(11, {sa_family=AF_INET6, sin6_port=htons(443), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "2001:67c:4e8:f004::9", &sin6_addr), sin6_scope_id=0}, 28) = -1 EINPROGRESS (Operation now in progress)
connect(9, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)
connect(9, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)
connect(9, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)
connect(9, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)
connect(9, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)

After this change

Connect once, use it as long as possible:

ammarfaizi2@integral2:~/p/ict-bot/build$ strace -e connect ./ict-bot
connect(7, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("149.154.167.220")}, 16) = -1 EINPROGRESS (Operation now in progress)

The `CurlHttpClient` currently produces an excessive number of
`connect()` syscalls, as every `makeRequest` creates a new TCP
connection. This inefficiency becomes particularly evident with
`TgLongPoll`, which continuously calls `getUpdates` in an endless loop,
creating a relentless cycle of `connect()` and `close()` operations.

A more sensible approach is to reuse HTTP connections by keeping them
alive across multiple requests. I suggest modifying the implementation
to allow the curl handle to persist until the `CurlHttpClient` object is
destroyed.

For thread safety, we can maintain a
`std::unordered_map<std::thread::id, CURL*> curlHandles` within the
`CurlHttpClient` class, ensuring each thread gets its own handle.

After all, treating connections like disposable napkins doesn't exactly
scream efficiency.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
@reo7sp
Copy link
Owner

reo7sp commented Jan 26, 2025

Thank you very much! Good job

@reo7sp reo7sp merged commit 3bef4dc into reo7sp:master Jan 26, 2025
1 check passed
ammarfaizi2 added a commit to ammarfaizi2/muika_bot that referenced this pull request Oct 5, 2025
The curl version is more efficient as it keeps the TCP socket
connection alive. See the link below for details.

Link: reo7sp/tgbot-cpp#331
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
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.

2 participants