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

Implement single connection benchmark #308

Merged
merged 2 commits into from
Nov 12, 2024

Conversation

sylwiaszunejko
Copy link
Collaborator

API for hooking to Write() and Read() of Conn struct added. New struct TrafficRecorder added for enabling recording and mocking single Connection flow.

Fixes: #296

@sylwiaszunejko
Copy link
Collaborator Author

Adding this as a draft, because I wanted to make sure this part looks like intended in the issue, before pushing the rest.

@sylwiaszunejko sylwiaszunejko self-assigned this Oct 14, 2024
@sylwiaszunejko sylwiaszunejko force-pushed the frame_serialization branch 2 times, most recently from 704bb86 to 6af5165 Compare October 15, 2024 08:17
@sylwiaszunejko
Copy link
Collaborator Author

@dkropachev could you take a look?

conn.go Outdated Show resolved Hide resolved
@sylwiaszunejko
Copy link
Collaborator Author

@dkropachev I pushed some changes, it is clearly wip, but I want to make sure the direction is right

scylla.go Outdated Show resolved Hide resolved
tests/bench/bench_conn_test.go Outdated Show resolved Hide resolved
tests/bench/bench_conn_test.go Outdated Show resolved Hide resolved
tests/bench/bench_conn_test.go Outdated Show resolved Hide resolved
scylla.go Outdated Show resolved Hide resolved
@sylwiaszunejko sylwiaszunejko force-pushed the frame_serialization branch 4 times, most recently from 30e27dc to 45191a1 Compare October 25, 2024 11:20
@sylwiaszunejko
Copy link
Collaborator Author

@dkropachev I pushed some changes, it is still not a complete solution, I am not sure how to replay the traffic properly, but it is in such a state that it should be possible to state if the direction is good

@sylwiaszunejko sylwiaszunejko force-pushed the frame_serialization branch 2 times, most recently from f75b620 to bf816cd Compare October 26, 2024 08:57
Copy link
Collaborator

@dkropachev dkropachev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably you could reuse something from TestServer from conn_test.go to replay traffic

dialer/replayer/replayer.go Outdated Show resolved Hide resolved
dialer/recorder/recorder.go Outdated Show resolved Hide resolved
dialer/recorder/recorder.go Outdated Show resolved Hide resolved
dialer/recorder/recorder.go Outdated Show resolved Hide resolved
dialer/recorder/recorder.go Outdated Show resolved Hide resolved
dialer/utils.go Outdated Show resolved Hide resolved
dialer/replayer/replayer.go Outdated Show resolved Hide resolved
dialer/replayer/replayer.go Outdated Show resolved Hide resolved
dialer/replayer/replayer.go Outdated Show resolved Hide resolved
dialer/replayer/replayer.go Outdated Show resolved Hide resolved
dialer/recorder/recorder.go Outdated Show resolved Hide resolved
dialer/recorder/recorder.go Outdated Show resolved Hide resolved
dialer/recorder/recorder.go Outdated Show resolved Hide resolved
dialer/utils.go Outdated Show resolved Hide resolved
dialer/recorder/recorder.go Outdated Show resolved Hide resolved
dialer/recorder/recorder.go Outdated Show resolved Hide resolved
dialer/recorder/recorder.go Outdated Show resolved Hide resolved
dialer/recorder/recorder.go Outdated Show resolved Hide resolved
dialer/replayer/replayer.go Outdated Show resolved Hide resolved
dialer/replayer/replayer.go Outdated Show resolved Hide resolved
dialer/replayer/replayer.go Outdated Show resolved Hide resolved
dialer/replayer/replayer.go Outdated Show resolved Hide resolved
dialer/replayer/replayer.go Outdated Show resolved Hide resolved
dialer/replayer/replayer.go Outdated Show resolved Hide resolved
@dkropachev
Copy link
Collaborator

@sylwiaszunejko , we need to disable these guys for NewSingleHostQueryExecutor

		s.hostSource = &ringDescriber{session: s}
		s.ringRefresher = newRefreshDebouncer(ringRefreshDebounceTime, func() error { return refreshRing(s.hostSource) })

dkropachev
dkropachev previously approved these changes Nov 6, 2024
dialer/replayer/replayer.go Outdated Show resolved Hide resolved
dialer/replayer/replayer.go Outdated Show resolved Hide resolved
dialer/replayer/replayer.go Outdated Show resolved Hide resolved
exec.go Show resolved Hide resolved
dialer/replayer/replayer.go Outdated Show resolved Hide resolved
dialer/recorder/recorder.go Outdated Show resolved Hide resolved
tests/bench/bench_single_conn_test.go Outdated Show resolved Hide resolved
dkropachev
dkropachev previously approved these changes Nov 8, 2024
Copy link
Collaborator

@dkropachev dkropachev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final change I it is finally ready, great job, it produces pretty stable results:

cpu: 12th Gen Intel(R) Core(TM) i9-12900HK
BenchmarkSingleConnectionInsert
BenchmarkSingleConnectionInsert/Insert
BenchmarkSingleConnectionInsert/Insert/Case0
BenchmarkSingleConnectionInsert/Insert/Case0-20           242354              4332 ns/op
BenchmarkSingleConnectionInsert/Insert/Case1
BenchmarkSingleConnectionInsert/Insert/Case1-20           295485              4334 ns/op
BenchmarkSingleConnectionInsert/Insert/Case2
BenchmarkSingleConnectionInsert/Insert/Case2-20           304800              4332 ns/op
BenchmarkSingleConnectionInsert/Insert/Case3
BenchmarkSingleConnectionInsert/Insert/Case3-20           266346              4240 ns/op
BenchmarkSingleConnectionInsert/Insert/Case4
BenchmarkSingleConnectionInsert/Insert/Case4-20           254448              4042 ns/op
BenchmarkSingleConnectionInsert/Insert/Case5
BenchmarkSingleConnectionInsert/Insert/Case5-20           288975              4270 ns/op
BenchmarkSingleConnectionInsert/Insert/Case6
BenchmarkSingleConnectionInsert/Insert/Case6-20           279169              4152 ns/op
BenchmarkSingleConnectionInsert/Insert/Case7
BenchmarkSingleConnectionInsert/Insert/Case7-20           294770              4074 ns/op
BenchmarkSingleConnectionInsert/Insert/Case8
BenchmarkSingleConnectionInsert/Insert/Case8-20           299186              3994 ns/op
BenchmarkSingleConnectionInsert/Insert/Case9
BenchmarkSingleConnectionInsert/Insert/Case9-20           305224              4147 ns/op
PASS

Add new types of dialers, one for recording one for
replaying the traffic. Record example traffic and then
use it to benchmark frame serialization and deserialization
without using real net.Conn.
@dkropachev
Copy link
Collaborator

@sylwiaszunejko , I spot another problem, standard benchrmark library does not collect goroutine stats, which means that some parts are not shown in results.
Unfortunately due to the library limitations, there is only one way to fix it is to come up with custom test library, which is a bit cumbersome.
Given all that I think it is better to merge it and look for solution that would show everything later, WDYT?

@sylwiaszunejko
Copy link
Collaborator Author

@sylwiaszunejko , I spot another problem, standard benchrmark library does not collect goroutine stats, which means that some parts are not shown in results. Unfortunately due to the library limitations, there is only one way to fix it is to come up with custom test library, which is a bit cumbersome. Given all that I think it is better to merge it and look for solution that would show everything later, WDYT?

I agree

@sylwiaszunejko
Copy link
Collaborator Author

I created an issue to collect goroutine stats #330

@sylwiaszunejko sylwiaszunejko merged commit d85e54e into scylladb:master Nov 12, 2024
1 check passed
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.

Benchmark frame serialization
2 participants