Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/net/http2: decrease memory consumption when small request coming #18509

Closed
ayanamist opened this issue Jan 4, 2017 · 9 comments
Closed

x/net/http2: decrease memory consumption when small request coming #18509

ayanamist opened this issue Jan 4, 2017 · 9 comments
Labels
FrozenDueToAge
Milestone
Go1.9

Comments

@ayanamist
Copy link

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.7.4 linux/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH=""
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build030072102=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"

What did you do?

package main

import (
	"fmt"
	"io"
	"io/ioutil"
	"log"
	"net"
	"net/http"
	"time"

	"golang.org/x/net/http2"
)

func main() {
	ln, err := net.Listen("tcp", ":8088")
	if err != nil {
		log.Fatal(err)
	}

	http.Handle("/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		n, _ := io.Copy(ioutil.Discard, r.Body)
		w.Write([]byte(fmt.Sprintf("%d from %v", n, r.Proto)))
	}))

	httpServer := &http.Server{
		ReadTimeout:  60 * time.Second,
		WriteTimeout: 60 * time.Second,
	}
	h2ServeOpts := &http2.ServeConnOpts{
		BaseConfig: httpServer,
	}
	h2Server := http2.Server{
		MaxReadFrameSize: 16 * 1024,
		IdleTimeout:      60 * time.Second,
	}

	for {
		conn, err := ln.Accept()
		if err != nil {
			log.Printf("accept: %v", err)
			continue
		}
		go h2Server.ServeConn(conn, h2ServeOpts)
	}
}

Use following to benchmark:
h2load -n 10000000 -c 1000 -m 100 -d ~/testdata http://127.0.0.1:8088/
~/testdata file is 950 bytes large.

What did you expect to see?

Use ps and top to see memory consumption, and not very large.

What did you see instead?

Memory consumption can go up to 8GB
image
@ayanamist
Copy link
Author

After some pprof investigate, i found the problem is related to reqBuf used in stream, i have some optimization for such small request (request body less than 1KB).

@gopherbot
Copy link

CL https://golang.org/cl/34754 mentions this issue.

@ayanamist
Copy link
Author

In our benchmark, after modification, heap size is no large than 1GB.

@rsc
Copy link
Contributor

rsc commented Jan 4, 2017

/cc @bradfitz

@rsc rsc added this to the Go1.9 milestone Jan 4, 2017
@tombergan
Copy link
Contributor

This is effectively a dup of #16512, especially this comment, which notes that we'll need a smarter buffering strategy to support connection-wide flow control. With the suggested limit of 1MB per connection from #16512, and 1000 concurrent connections (h2load -c 1000 ...), that is a max of 1GB total, same as in the above comment.

@ayanamist
Copy link
Author

@tombergan Although the result is similar with #16512, the implementation is quite different, i do not use any per connection memory limit, but use existing Content-Length header from requests. If i change the size of testdata to smaller like 100 bytes or larger like 4KB, the heap size will change too.
In fact there are many similar memory wasting, and this is the easiest modification i found.

@tombergan
Copy link
Contributor

Yes, thanks for the report. We need to rethink the buffering strategy to better share the connection's flow window across all requests. As you point out, we should minimize the amount of memory allocated to individual requests, rather than blindly allocating 64KB to every request. However, I think we'd implement that slightly differently than in your sample CL, by trying to do a better job of reusing buffers for multiple requests so each POST doesn't require an allocation.

@ayanamist
Copy link
Author

@tombergan I am very looking foward to your job, we have a very h2 traffic and quite need any cpu and memory optimization.

@gopherbot
Copy link

CL https://golang.org/cl/37400 mentions this issue.

@golang golang locked and limited conversation to collaborators Feb 24, 2018
c3mb0 pushed a commit to c3mb0/net that referenced this issue Apr 2, 2018
@tombergan
http2: replace fixedBuffer with dataBuffer
788e740 
fixedBuffer was a bad idea for two reasons:

1. It was fixed at a constant 64KB (the current default flow-control
   window) which wastes memory on the server when clients upload many
   small request bodies.

2. A follow-up CL will allow configuring the server's connection and
   stream receive windows. We want to allow individual streams to use
   varying amounts of the available connection window. This is not
   possible when each stream uses a fixedBuffer.

dataBuffer grows and shrinks based on current usage. The worst-case
fragmentation of dataBuffer is 32KB wasted memory per stream, but I
expect that worst-case will be rare. In particular, if the declared
size of a stream's request body is under 1KB, then the server will not
allocate more than 1KB to process that stream's request body.

Updates golang/go#16512
Fixes golang/go#18509

Change-Id: Ibcb18007037e82518a65848ef3baf4937955ac9d
Reviewed-on: https://go-review.googlesource.com/37400
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.9
Development

No branches or pull requests
4 participants
@rsc @ayanamist @gopherbot @tombergan