Skip to content

Commit

Permalink
Reduce allocations in HTTP client lookups
Browse files Browse the repository at this point in the history
  • Loading branch information
neilalexander committed Apr 14, 2024
1 parent 4e20924 commit f8c00fe
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 46 deletions.
6 changes: 3 additions & 3 deletions internal/gomatrixserverlib/fclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand Down
70 changes: 28 additions & 42 deletions internal/gomatrixserverlib/fclient/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion internal/gomatrixserverlib/fclient/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit f8c00fe

Please sign in to comment.