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

x/crypto/ssh: performance improvement #39117

Open
drakkan opened this issue May 17, 2020 · 0 comments
Open

x/crypto/ssh: performance improvement #39117

drakkan opened this issue May 17, 2020 · 0 comments

Comments

@drakkan
Copy link

@drakkan drakkan commented May 17, 2020

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

$ go version
go version go1.14.2 linux/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/nicola/.cache/go-build"
GOENV="/home/nicola/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/nicola/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build437742805=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I'm using crypto/ssh for SFTP inside SFTPGo (https://github.com/drakkan/sftpgo). The performance are generally good and very similar to OpenSSH for SFTP downloads. SFTP uploads are a bit slower than OpenSSH.

Here is a profiling result:

(pprof) top                                                       
Showing nodes accounting for 673.49MB, 97.46% of 691.07MB total
Dropped 51 nodes (cum <= 3.46MB)
Showing top 10 nodes out of 45
      flat  flat%   sum%        cum   cum%
  603.99MB 87.40% 87.40%   603.99MB 87.40%  golang.org/x/crypto/ssh.(*connectionState).readPacket
      64MB  9.26% 96.66%       64MB  9.26%  golang.org/x/crypto/argon2.initBlocks
       5MB  0.72% 97.38%        8MB  1.16%  golang.org/x/crypto/ssh.Marshal (inline)
    0.50MB 0.072% 97.46%     8.50MB  1.23%  golang.org/x/crypto/ssh.(*channel).adjustWindow
         0     0% 97.46%     3.61MB  0.52%  crypto/x509.SystemCertPool
         0     0% 97.46%     3.61MB  0.52%  crypto/x509.initSystemRoots
         0     0% 97.46%     3.61MB  0.52%  crypto/x509.loadSystemRoots
         0     0% 97.46%     3.61MB  0.52%  crypto/x509.systemRootsPool (inline)
         0     0% 97.46%       64MB  9.26%  github.com/alexedwards/argon2id.ComparePasswordAndHash
         0     0% 97.46%     3.61MB  0.52%  github.com/drakkan/sftpgo/cmd.Execute
(pprof) list readPacket
Total: 691.07MB
....
  603.99MB   603.99MB    163:	fresh := make([]byte, len(packet))
         .          .    164:	copy(fresh, packet)
         .          .    165:
         .          .    166:	return fresh, err
         .          .    167:}
         .          .    168:

so we can improve upload performance avoid this allocation

What did you expect to see?

Data upload with minimal memory allocation to improve performance

What did you see instead?

a new slice is allocated for each received packet

I can try to write a patch but I would like some suggestions on the best approach to do this.
I'm thinking about a per connection circular ring buffer of slices with a configurable max size, so we can reuse the preallocated slices once the ring buffer is filled.
What do you think about? Do you have other suggestions? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.