From f8c00fe6fc3412599692b180273ab98a49c3af38 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sun, 14 Apr 2024 19:49:48 +0100 Subject: [PATCH] Reduce allocations in HTTP client lookups --- internal/gomatrixserverlib/fclient/client.go | 6 +- internal/gomatrixserverlib/fclient/resolve.go | 70 ++++++++----------- .../gomatrixserverlib/fclient/resolve_test.go | 3 +- 3 files changed, 33 insertions(+), 46 deletions(-) diff --git a/internal/gomatrixserverlib/fclient/client.go b/internal/gomatrixserverlib/fclient/client.go index b3606ad3..1e0a1d0d 100644 --- a/internal/gomatrixserverlib/fclient/client.go +++ b/internal/gomatrixserverlib/fclient/client.go @@ -257,7 +257,8 @@ func (f *destinationTripper) RoundTrip(r *http.Request) (*http.Response, error) var err error serverName := spec.ServerName(r.URL.Host) resolutionRetried := false - resolutionResults := []ResolutionResult{} + var _resolutionResults [16]ResolutionResult + resolutionResults := _resolutionResults[:0] retryResolution: if f.wellKnownSRV { @@ -270,8 +271,7 @@ retryResolution: // If the cache returned nothing then we'll have no results here, // so go and hit the network. if len(resolutionResults) == 0 { - resolutionResults, err = ResolveServer(r.Context(), serverName) - if err != nil { + if err = ResolveServer(r.Context(), serverName, &resolutionResults); err != nil { return nil, err } f.resolutionCache.Store(serverName, resolutionResults) diff --git a/internal/gomatrixserverlib/fclient/resolve.go b/internal/gomatrixserverlib/fclient/resolve.go index 024cc2b1..70676570 100644 --- a/internal/gomatrixserverlib/fclient/resolve.go +++ b/internal/gomatrixserverlib/fclient/resolve.go @@ -37,18 +37,17 @@ type ResolutionResult struct { // Returns a slice of ResolutionResult that can be used to send a federation // request to the server using a given server name. // Returns an error if the server name isn't valid. -func ResolveServer(ctx context.Context, serverName spec.ServerName) (results []ResolutionResult, err error) { - return resolveServer(ctx, serverName, true) +func ResolveServer(ctx context.Context, serverName spec.ServerName, results *[]ResolutionResult) error { + return resolveServer(ctx, serverName, true, results) } // resolveServer does the same thing as ResolveServer, except it also requires // the checkWellKnown parameter, which indicates whether a .well-known file // should be looked up. -func resolveServer(ctx context.Context, serverName spec.ServerName, checkWellKnown bool) (results []ResolutionResult, err error) { +func resolveServer(ctx context.Context, serverName spec.ServerName, checkWellKnown bool, results *[]ResolutionResult) error { host, port, valid := spec.ParseAndValidateServerName(serverName) if !valid { - err = fmt.Errorf("Invalid server name") - return + return fmt.Errorf("Invalid server name") } // 1. If the hostname is an IP literal @@ -66,50 +65,42 @@ func resolveServer(ctx context.Context, serverName spec.ServerName, checkWellKno destination = string(serverName) } - results = []ResolutionResult{ - { - Destination: destination, - Host: serverName, - TLSServerName: host, - }, - } - - return + *results = append(*results, ResolutionResult{ + Destination: destination, + Host: serverName, + TLSServerName: host, + }) + return nil } // 2. If the hostname is not an IP literal, and the server name includes an // explicit port if port != -1 { - results = []ResolutionResult{ - { - Destination: string(serverName), - Host: serverName, - TLSServerName: host, - }, - } - - return + *results = append(*results, ResolutionResult{ + Destination: string(serverName), + Host: serverName, + TLSServerName: host, + }) + return nil } if checkWellKnown { // 3. If the hostname is not an IP literal - var result *WellKnownResult - result, err = LookupWellKnown(ctx, serverName) - if err == nil { + if result, err := LookupWellKnown(ctx, serverName); err == nil { // We don't want to check .well-known on the result - return resolveServer(ctx, result.NewAddress, false) + return resolveServer(ctx, result.NewAddress, false, results) } } - return handleNoWellKnown(ctx, serverName), nil + handleNoWellKnown(ctx, serverName, results) + return nil } // handleNoWellKnown implements steps 4 and 5 of the resolution algorithm (as // well as 3.3 and 3.4) -func handleNoWellKnown(ctx context.Context, serverName spec.ServerName) (results []ResolutionResult) { +func handleNoWellKnown(ctx context.Context, serverName spec.ServerName, results *[]ResolutionResult) { // 4. If the /.well-known request resulted in an error response - records, err := lookupSRV(ctx, serverName) - if err == nil && len(records) > 0 { + if records, err := lookupSRV(ctx, serverName); err == nil && len(records) > 0 { for _, rec := range records { // If the domain is a FQDN, remove the trailing dot at the end. This // isn't critical to send the request, as Go's HTTP client and most @@ -120,27 +111,22 @@ func handleNoWellKnown(ctx context.Context, serverName spec.ServerName) (results target = target[:len(target)-1] } - results = append(results, ResolutionResult{ + *results = append(*results, ResolutionResult{ Destination: fmt.Sprintf("%s:%d", target, rec.Port), Host: serverName, TLSServerName: string(serverName), }) } - return } // 5. If the /.well-known request returned an error response, and the SRV // record was not found - results = []ResolutionResult{ - { - Destination: fmt.Sprintf("%s:%d", serverName, 8448), - Host: serverName, - TLSServerName: string(serverName), - }, - } - - return + *results = append(*results, ResolutionResult{ + Destination: fmt.Sprintf("%s:%d", serverName, 8448), + Host: serverName, + TLSServerName: string(serverName), + }) } func lookupSRV(ctx context.Context, serverName spec.ServerName) ([]*net.SRV, error) { diff --git a/internal/gomatrixserverlib/fclient/resolve_test.go b/internal/gomatrixserverlib/fclient/resolve_test.go index 851c3416..fd72047e 100644 --- a/internal/gomatrixserverlib/fclient/resolve_test.go +++ b/internal/gomatrixserverlib/fclient/resolve_test.go @@ -28,7 +28,8 @@ func assertCritical(t *testing.T, val, expected interface{}) { // If one of them doesn't match, or the resolution function returned with an // error, it aborts the current test. func testResolve(t *testing.T, serverName spec.ServerName, destination, host, certName string) { - res, err := ResolveServer(context.Background(), serverName) + res := []ResolutionResult{} + err := ResolveServer(context.Background(), serverName, &res) assertCritical(t, err, nil) assertCritical(t, len(res), 1) assertCritical(t, res[0].Destination, destination)