perf(reader): reuse TCP connection across well-known feed URL probes#4342
Draft
jvoisin wants to merge 1 commit into
Draft
perf(reader): reuse TCP connection across well-known feed URL probes#4342jvoisin wants to merge 1 commit into
jvoisin wants to merge 1 commit into
Conversation
Collaborator
Author
|
I'm not that happy about the implementation, but I think it's the least worst one. |
dsh2dsh
added a commit
to dsh2dsh/miniflux
that referenced
this pull request
May 19, 2026
gudvinr
reviewed
Jun 13, 2026
gudvinr
reviewed
Jun 13, 2026
gudvinr
reviewed
Jun 13, 2026
findSubscriptionsFromWellKnownURLs sequentially probes a list of well-known feed paths against a single host. Each probe went through ExecuteRequest, which builds a fresh *http.Client, sends "Connection: close", and tears the TCP/TLS connection down, meaning the N probes paid N TLS handshakes against the same host. This commit changes ExecuteRequest and adds a Close method to RequestBuilder, with the client being (optionally) reused by later calls, so the underlying TCP/TLS connection (and HTTP/2 stream multiplexing) can be reused. For example, the finder calls ExecuteRequest in the probe loop and defers Close to release the pool. There is also a drive-by fix of an issue in the request path: per-request header tweaks (Accept-Encoding, Accept, Connection) were written directly into the builder's shared headers map, leaking state back into the builder and racing for any concurrent caller. The header map is now cloned before mutating it. Of course, tests were added :) Reviewed-by: gudvinr
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
findSubscriptionsFromWellKnownURLs sequentially probes a list of well-known feed paths against a single host. Each probe went through ExecuteRequest, which builds a fresh *http.Client, sends "Connection: close", and tears the TCP/TLS connection down, meaning the N probes paid N TLS handshakes against the same host.
This commit adds a ExecuteRequestKeepalive and a Close method to RequestBuilder. The keep-alive client is lazily built on first use and stashed on the builder; later calls reuse it, so the underlying TCP/TLS connection (and HTTP/2 stream multiplexing) can be reused. The finder calls ExecuteRequestKeepalive in the probe loop and defers Close to release the pool.
There is also a drive-by fix of an issue in the request path: per-request header tweaks (Accept-Encoding, Accept, Connection) were written directly into the builder's shared headers map, leaking state back into the builder and racing for any concurrent caller. The header map is now cloned before mutating it.
Of course, tests were added :)