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.

Sign up for GitHub

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

add tests for ReceiveFrom and SendTo #3211

Merged
merged 3 commits into from
Sep 28, 2023
Merged

add tests for ReceiveFrom and SendTo #3211

merged 3 commits into from
Sep 28, 2023

Conversation

wfurt
Copy link
Member

@wfurt wfurt commented Aug 1, 2023

@wfurt wfurt self-assigned this Aug 1, 2023
@cincuranet
Copy link
Contributor

@wfurt is this ready to be merged? RC1 is now in the wild, so maybe the comment above from @stephentoub can be resolved. I'd like to have this merged.

@wfurt
Copy link
Member Author

wfurt commented Sep 26, 2023

updated.

| Method                                         | Mean         | Error       | StdDev      | Median       | Min           | Max          | Allocated |
|----------------------------------------------- |-------------:|------------:|------------:|-------------:|--------------:|-------------:|----------:|
| ConnectAcceptAsync                             |     1.247 ms |   0.3023 ms |   0.3482 ms |     1.010 ms |     0.9320 ms |     2.149 ms |    1344 B |
| SendAsyncThenReceiveAsync_Task                 |   158.350 ms |  15.9768 ms |  17.0950 ms |   152.039 ms |   139.9396 ms |   197.956 ms |     752 B |
| ReceiveAsyncThenSendAsync_Task                 | 2,580.502 ms | 346.1255 ms | 398.5986 ms | 2,521.235 ms | 2,095.5751 ms | 3,395.453 ms |     992 B |
| SendAsyncThenReceiveAsync_SocketAsyncEventArgs |   229.977 ms |  34.1303 ms |  35.0493 ms |   217.763 ms |   202.6055 ms |   334.964 ms |    1904 B |
| ReceiveAsyncThenSendAsync_SocketAsyncEventArgs | 2,717.777 ms | 241.8179 ms | 258.7424 ms | 2,631.813 ms | 2,393.2247 ms | 3,469.581 ms |    3072 B |
| ReceiveFromAsyncThenSendToAsync_Task           | 4,171.189 ms | 391.4736 ms | 402.0146 ms | 4,176.913 ms | 3,668.5098 ms | 5,079.582 ms | 4321360 B |
| ReceiveFromAsyncThenSendToAsync_SocketAddress  | 4,072.631 ms | 333.8839 ms | 357.2520 ms | 3,970.266 ms | 3,681.9061 ms | 4,907.698 ms |    1432 B |
| SendToThenReceiveFrom                          |   231.212 ms |   4.6446 ms |   5.1625 ms |   231.196 ms |   222.4927 ms |   242.110 ms | 4320784 B |

ReceiveFromAsyncThenSendToAsync_SocketAddress showing great allocation improvement ... as it should.

@wfurt
Copy link
Member Author

wfurt commented Sep 26, 2023

Blazor test failures seems unrelated.

@cincuranet
Copy link
Contributor

@stephentoub Is this OK from your POV to merge?

stephentoub
stephentoub reviewed Sep 28, 2023
View reviewed changes
src/benchmarks/micro/libraries/System.Net.Sockets/Perf.Socket.cs
@@ -116,6 +118,67 @@ public async Task ReceiveAsyncThenSendAsync_SocketAsyncEventArgs()
}
}

[Benchmark]
public async Task ReceiveFromAsyncThenSendToAsync_Task()
Copy link
Member

Choose a reason for hiding this comment

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

Nit: should this be named _EndPoint rather than _Task?

stephentoub
stephentoub reviewed Sep 28, 2023
View reviewed changes
src/benchmarks/micro/libraries/System.Net.Sockets/Perf.Socket.cs
}

[Benchmark]
public void SendToThenReceiveFrom()
Copy link
Member

Choose a reason for hiding this comment

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

You added async APIs for both EndPoint and SocketAddress, but sync for only EndPoint?

@cincuranet cincuranet merged commit a1ab22d into main Sep 28, 2023
26 of 30 checks passed
@adamsitnik adamsitnik mentioned this pull request Nov 8, 2023
Merged
@LoopedBard3 LoopedBard3 deleted the receiveFrom branch March 11, 2024 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stephentoub stephentoub stephentoub approved these changes

@antonfirsov antonfirsov Awaiting requested review from antonfirsov

@liveans liveans Awaiting requested review from liveans

Assignees

@wfurt wfurt

Labels
adds new benchmarks
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wfurt @cincuranet @stephentoub