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

Port benchmarks for the websocket encoder/decoder. #12782

Merged
merged 3 commits into from
Sep 13, 2022

Conversation

amizurov
Copy link
Sponsor Contributor

@amizurov amizurov commented Sep 8, 2022

Motivation:

Porting benchmarks for the websocket encoder/decoder.

Modification:

Add WebSocketFrame13DecoderBenchmark and WebSocketFrame13EncoderBenchmark benchmarks.

Result:

We can  compare changes related to websocket performance.


@Benchmark
public Future<Void> writeWebSocketFrame() {
return websocketEncoder.write(context, new BinaryWebSocketFrame(content.split()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Use allocator.constBufferSupplier in the setUp, because after the first call to split, the content buffer will be empty.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same in the decoder benchmark.

Copy link
Sponsor Contributor Author

@amizurov amizurov Sep 8, 2022

Choose a reason for hiding this comment

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

@chrisvest Thanks, got it. Hmm, i also see that EmbeddedEventLoop has changed its behavior. In 4.1 it always returns true when calling inEventLoop(), now only if a task running inside the current loop. I think we need to review all benchmarks because now they may not work correctly.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

@chrisvest PTAL again

@amizurov
Copy link
Sponsor Contributor Author

@chrisvest @normanmaurer As i mentioned the EmbededEventLoop has changed its behaviour and i'm not found a better solution than executing benchmark inside an embedded event loop to proper handling writePromiseCombiner(ctx, out, promise). Maybe you can advice something else. Also bellow the benchmark results according changes (#12780) the cost of split() is significantly decrease performance.

Screenshot 2022-09-12 at 18 53 53

Screenshot 2022-09-12 at 18 53 42

@chrisvest
Copy link
Contributor

The build failed because it needs a rebase to fix imports.

I don't see split showing up much in the profile. Rather, it's the always-on cleaners; CleanerDrop.innerWrap accounts for 17.6% of the time in a run I just did. I plan to make them off by default, and that should bring performance much closer.

@amizurov amizurov closed this Sep 12, 2022
@amizurov amizurov reopened this Sep 12, 2022
@amizurov
Copy link
Sponsor Contributor Author

The build failed because it needs a rebase to fix imports.

I don't see split showing up much in the profile. Rather, it's the always-on cleaners; CleanerDrop.innerWrap accounts for 17.6% of the time in a run I just did. I plan to make them off by default, and that should bring performance much closer.

Thanks, I'll be happy to double-check after disabling, because it's inconvenient to manually close the incoming buffer in codecs.

@chrisvest
Copy link
Contributor

I played around a bit; the smaller the contentLength, the bigger impact CleanerDrop.innerWrap has. 43.9% with contentLength = 100. The 17% was from contentLength = 3000. Using off-heap allocator and masking = false.

@chrisvest chrisvest merged commit 329c392 into netty:main Sep 13, 2022
@chrisvest
Copy link
Contributor

Thanks

@chrisvest chrisvest added this to the 5.0.0.Alpha5 milestone Sep 13, 2022
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.

None yet

2 participants