Skip to content

Commit

Permalink
Drain Response.Body to enable TCP/TLS connection reuse (4x speedup)
Browse files Browse the repository at this point in the history
On successful queries (on errors ReadAll runs in CheckResponse), the
Response.Body is not read all the way to the EOF as json.Decoder is used,
which stops at the end of the object, which is probably followed by a \n.

Then, Response.Body.Close() is called.  When that happens with a non-drained
Body, the TCP/TLS connection is closed (which makes sense, in case there was
still a lot to read from the network), stopping the Transport from reusing
it.  Also, a library user can't drain the Body themselves because Close() is
called before passing the Response to the user.

Since we know GitHub API responses only carry a single JSON object, draining
before closing is free, and saving all those handshakes causes huge savings
in latency, bandwidth and CPU for both the client and the poor GitHub
servers.

Here's the result of running the benchmark below on a ADSL connection:

    before: 2m3.001599093s
    after: 33.753543928s

    func main() {
    	ts := oauth2.StaticTokenSource(
    		&oauth2.Token{AccessToken: os.Getenv("GITHUB_TOKEN")},
    	)
    	tc := oauth2.NewClient(oauth2.NoContext, ts)
    	gh := github.NewClient(tc)
    	start := time.Now()
    	for i := 0; i < 100; i++ {
    		gh.Repositories.Get("FiloSottile", "gvt")
    		fmt.Print(".")
    	}
    	fmt.Printf("\n%s\n", time.Now().Sub(start))
    }
  • Loading branch information
FiloSottile committed Mar 27, 2016
1 parent 2741d95 commit e0ff711
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,11 @@ func (c *Client) Do(req *http.Request, v interface{}) (*Response, error) {
return nil, err
}

defer resp.Body.Close()
defer func() {
// Drain and close the body to let the Transport reuse the connection
io.Copy(ioutil.Discard, resp.Body)
resp.Body.Close()
}()

response := newResponse(resp)

Expand Down

0 comments on commit e0ff711

Please sign in to comment.