PATCH: PR27701: debuginfod clients with long-lived curl handles
Sat May 1 00:03:48 GMT 2021
On Fri, Apr 23, 2021 at 03:43:09PM -0400, Frank Ch. Eigler via Elfutils-devel wrote:
> PR27701: debuginfod client: encourage reused debuginfod_client objects
> Client objects now carry long-lived curl handles for outgoing
> connections. This makes it more efficient for multiple sequential
> queries, because the TCP connections and/or TLS state info are kept
> around awhile, avoiding O(100ms) setup latencies. debuginfod is
> adjusted to take advantage of this for federation. Other clients
> should gradually do this too, perhaps including elfutils itself (in
> the libdwfl->debuginfod_client hooks).
> A large gdb session with 117 debuginfo downloads was observed to run
> twice as fast (45s vs. 1m30s wall-clock time), just in nuking this
> extra setup latency. This was tested via a debuginfod intermediary:
> it should be even faster once gdb reuses its own debuginfod_client.
This is really nice. We should indeed also reuse a debuginfod_client
handle in libdwfl for a Dwfl or Dwfl_Module when we figure
out/document the multi-thread use case. The way debuginfod uses it is
multi-thread safe since it makes sure only one thread at a time uses
the same debuginfod_client handle.
The code looks good. I have only two comments:
> @@ -1095,6 +1087,11 @@ debuginfod_query_server (debuginfod_client *c,
> /* general purpose exit */
> + /* Reset sent headers */
> + curl_slist_free_all (c->headers);
> + c->headers = NULL;
> + c->user_agent_set_p = 0;
> /* Conclude the last \r status line */
> /* Another possibility is to use the ANSI CSI n K EL "Erase in Line"
> code. That way, the previously printed messages would be erased,
> @@ -175,8 +177,8 @@ of the client object.
> .SS "HTTP HEADER"
> -Before a lookup function is initiated, a client application may
> -add HTTP request headers to future downloads.
> +Before each lookup function is initiated, a client application may
> +add HTTP request headers. These are reset after each lookup function.
Why do we need to reset the headers? Is that because we don't have
debuginfod_remove_http_header? Should we maybe add that (or a
> @@ -90,10 +113,6 @@ wait_ready()
> if [ $timeout -eq 0 ]; then
> - echo "metric $what never changed to $value on port $port"
I think we want to keep this message ^ it helps with knowing which
exact metric timed out.
> - curl -s http://127.0.0.1:$port/metrics
> - echo "logs for debuginfod with port $port"
> - cat vlog$port
> exit 1;
More information about the Elfutils-devel