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

0gc #443

Merged
merged 6 commits into from Feb 25, 2019

Conversation

Projects
None yet
4 participants
@vis2k
Copy link
Owner

commented Feb 25, 2019

no-gc branch.
best to look at each separate commit. best to merge without squash.
note the SendToReady/All/Observers changes: previously the message was constructed for EACH connection that we send to. now it is constructed ONCE. that is a giant improvement.

stress test with uMMORPG and 420 CCU, 16 core server.
before:
image

0gc, 50hz, networkmessage as class:
image
=> 2/3 CPU% at 100 CCU
=> 1/2 CPU% at 200 CCU
=> 1/3 latency at 420 CCU
=> 420 CCU is where main thread is at around full load, where as it was under full load at 200 CCU before

vis2k added some commits Feb 24, 2019

Protocol.PackMessage caches writer and takes message as parameter to …
…serialize it directly into the writer after writing the message type.
NetworkServer.SendToReady/All/Observers: only pack message once inste…
…ad of repacking it for each one again. This should avoid giant amounts of allocations (PackMessage->Writer.ToArray()) and computations

@vis2k vis2k requested a review from paulpach Feb 25, 2019

@paulpach
Copy link
Collaborator

left a comment

Very nice!

the SendToReady/All/Observers will particularly make a huge difference when there are a lot of players together.

There are still other low hanging fruit. The Barriers patch addresses one, there is another easy one in the NetworkWriter. Each commit is nicely self contained so thumbs up for merging without squash

@vis2k vis2k merged commit 7455199 into master Feb 25, 2019

2 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@paulpach paulpach deleted the 0gc branch Feb 25, 2019

@rodolphito

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2019

Maybe I didn't understand the screenshots correctly, but it seems like they are testing different things? The CCU counts on each row dont line up. Before is: 100 -> 200 -> 450 -> 420, and After never tests 450, it only goes 100 -> 200 -> 320 -> 400 -> 420. Also, great job, I'm really happy attention is being placed here.

@SoftwareGuy

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2019

@rodolphito I think you had this explained in the Discord? I think 450 was attempted but the bot machine died, so he went back to the last safe value of 420 bots.

@vis2k

This comment has been minimized.

Copy link
Owner Author

commented Mar 19, 2019

🎉 This PR is included in version 1.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@vis2k vis2k added the released label Mar 19, 2019

tomkrikorian pushed a commit to tomkrikorian/Mirror that referenced this pull request Jun 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.