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

Reduce tds buffer size to match packetsize #584

Merged
merged 1 commit into from
Nov 3, 2020

Conversation

sysadmind
Copy link
Contributor

We noticed that when connecting to many databases, the amount of memory being utilized by the newTdsBuffer() func was rather significant, especially in comparison to the business logic that utilized the connections. I was able to use pprof to figure out that the newTdsBuffer() func was the source of the memory utilization and that these 2 buffers get initialized to a fairly large size. This reduces the size of the buffers to match the packetsize, significantly reducing the amount of memory used for the buffers in the majority use case.

I added a benchmark in order to help illustrate the difference in utilization before and after. Below are
the results on my machine before and after this change. In the default case (packetSize = 4096) this resulted in a 64% reduction in bytes allocated per operation. This number includes the overall execution and checking of the results as they are inside the benchmark .Run()

Before:

Running tool: /usr/lib/go-1.14/bin/go test -benchmem -run=^$ github.com/denisenkom/go-mssqldb -bench ^(BenchmarkPacketSize)$ -count=1 -race
goos: linux
goarch: amd64
pkg: github.com/denisenkom/go-mssqldb
BenchmarkPacketSize/PacketSize_2048-8         	     170	   6942432 ns/op	  193572 B/op	     734 allocs/op
BenchmarkPacketSize/PacketSize_4096-8         	     174	   6939035 ns/op	  193742 B/op	     735 allocs/op
BenchmarkPacketSize/PacketSize_8192-8         	     170	   6873276 ns/op	  193766 B/op	     735 allocs/op
BenchmarkPacketSize/PacketSize_16384-8        	     170	   6975863 ns/op	  194005 B/op	     736 allocs/op
PASS
ok  	github.com/denisenkom/go-mssqldb	7.602s

After:

Running tool: /usr/lib/go-1.14/bin/go test -benchmem -run=^$ github.com/denisenkom/go-mssqldb -bench ^(BenchmarkPacketSize)$ -count=1 -race
goos: linux
goarch: amd64
pkg: github.com/denisenkom/go-mssqldb
BenchmarkPacketSize/PacketSize_2048-8         	     170	   6837788 ns/op	   66434 B/op	     733 allocs/op
BenchmarkPacketSize/PacketSize_4096-8         	     178	   7019267 ns/op	   70413 B/op	     734 allocs/op
BenchmarkPacketSize/PacketSize_8192-8         	     170	   6654027 ns/op	   78992 B/op	     735 allocs/op
BenchmarkPacketSize/PacketSize_16384-8        	     176	   6586988 ns/op	   95226 B/op	     734 allocs/op
PASS
ok  	github.com/denisenkom/go-mssqldb	7.524s

The tests look good locally after the change except for 2 of the TVP tests, but those also failed before the change so I don't believe this change introduces any regressions there.

The *testing.T pointers have been changed in a few places to the interface testing.TB which both testing.T and testing.B implement to allow for using both.

We noticed that when connecting to many databases, the amount of memory being utilized by the newTdsBuffer() func was rather significant, especially in comparison to the business logic that utilized the connections. I was able to use pprof to figure out that the newTdsBuffer() func was the source of the memory utilization and that these 2 buffers get initialized to a fairly large size. This reduces the size of the buffers to match the packetsize, significantly reducing the amount of memory used for the buffers in the majority use case.
@codecov
Copy link

codecov bot commented Jun 22, 2020

Codecov Report

Merging #584 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #584   +/-   ##
=======================================
  Coverage   68.72%   68.72%           
=======================================
  Files          23       23           
  Lines        5190     5190           
=======================================
  Hits         3567     3567           
  Misses       1410     1410           
  Partials      213      213           
Impacted Files Coverage Δ
buf.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b91950f...c2a4f36. Read the comment docs.

@denisenkom denisenkom merged commit e321924 into denisenkom:master Nov 3, 2020
ninjadq pushed a commit to ninjadq/go-mssqldb that referenced this pull request May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants