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

Improve performance of NetworkSession.SendBundle #4207

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

FlaggAC
Copy link
Contributor

@FlaggAC FlaggAC commented Jul 25, 2024

Notably, the performance improvement is much greater when the packet back-pressure is high. This is likely due to the previous implementation of pruning items from a removal list.

The number with each benchmark is the number of messages in the bundle.

The message used for benchmarking:
new GameMessagePlayerKilled("Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum", new ObjectGuid(0x80000000), new ObjectGuid(0x80000001))

I would advise some kind of extensive testing before releasing. I only made sure to do a few basic functions ingame.

BenchmarkDotNet v0.13.12, Windows 11 (10.0.22631.3880/23H2/2023Update/SunValley3)
Intel Core i9-10850K CPU 3.60GHz, 1 CPU, 20 logical and 10 physical cores
.NET SDK 8.0.303
  [Host]     : .NET 8.0.7 (8.0.724.31311), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.7 (8.0.724.31311), X64 RyuJIT AVX2


| Method              | Mean            | Error         | StdDev        | Median          | Gen0    | Gen1   | Allocated |
|-------------------- |----------------:|--------------:|--------------:|----------------:|--------:|-------:|----------:|
| SendBundle_Old_1    |        62.64 ns |      1.165 ns |      1.090 ns |        62.61 ns |  0.0275 |      - |     288 B |
| SendBundle_Old_10   |     1,536.86 ns |     30.206 ns |     73.525 ns |     1,525.48 ns |  0.5379 | 0.0019 |    5632 B |
| SendBundle_Old_100  |    26,285.28 ns |    518.723 ns |  1,149.454 ns |    26,516.33 ns |  5.0659 | 0.0916 |   52992 B |
| SendBundle_Old_1000 | 1,388,448.45 ns | 26,634.106 ns | 30,671.876 ns | 1,374,837.99 ns | 48.8281 | 5.8594 |  518129 B |
| SendBundle_New_1    |        52.74 ns |      1.065 ns |      2.000 ns |        52.23 ns |  0.0268 |      - |     280 B |
| SendBundle_New_10   |     1,101.58 ns |     22.005 ns |     59.492 ns |     1,084.54 ns |  0.4654 | 0.0019 |    4880 B |
| SendBundle_New_100  |    10,116.14 ns |    191.578 ns |    255.751 ns |    10,037.81 ns |  4.4250 | 0.0610 |   46344 B |
| SendBundle_New_1000 |   101,756.68 ns |  2,029.821 ns |  3,861.946 ns |   100,630.79 ns | 43.5791 | 5.6152 |  456752 B |

Comment on lines -913 to -914
if (packet.Data != null)
availableSpace -= (int)packet.Data.Length;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These lines were removed because they have no effect

@FlaggAC FlaggAC force-pushed the rf/performance/send-bundle branch from 99f5474 to 9fac991 Compare July 25, 2024 02:38
@gmriggs
Copy link
Collaborator

gmriggs commented Jul 25, 2024

thanks for all of these performance improvements, great stuff!

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