Expiremental Perf Code#202
Conversation
|
Good to see another PR from you, friend! Again, absolutely no opposition from me on the shuffle change. Looks good. Hash seems fine too; I haven't checked out the lib yet, but surface level looks fine. I've flagged Aleksey -- I know he's been very, very busy lately with decreasing time availability but hopefully he'll have a chance to give this a peak. |
|
We recently promoted this to our production server instead of testing (using 1.13.2), and our performance seems to be doing better. It's only been a couple hours, but we'll let you know if anything happens after running this with actual players who aren't testers for a few days. |
|
Thank you so much again! |
|
I'm hoping some of this widespread crashing is now resolved, so will look to merge in this next :) |
|
Applied here 5.0.0 Release |
This commit includes two performance fixes that have been beneficial on our server. However, these changes on one server may not be the experience for everyone (and thus be beneficial to merge into upstream). We hope by opening this PR we can get some more people to test locally, and (maybe even on their server)!
The two changes we provide in this (besides a pom.xml change for ProtocolLib so it can build locally on a fresh box where the dependencies aren't cached) are:
Collections.shuffle. The reason for using fisher-yates instead ofCollections.shufflewere described originally when I was working on the original 13.1 PR, but I'll copy it here for those who don't want to go find it or didn't follow it):All in all it means one less place we allocate objects that just get GC'd on the next GC run, while still keeping the O(N) nature of
Collections.shuffle(so we're not taking a perf hit in time, just helping GC run faster!)Although CRC-32 is relatively fast when compared to something like MD5, it still isn't really what we're looking for. Which is a non cryptographic high throughput hash. For here we turn to xxHash (specifically xxHash64), which is considered one of the fastest hashes available without sacrificing quality. This statement is specifically coming from multiple SMHasher (the de-facto hash tester these days) results. 1 2 3 4.
Obviously though what everyone else tests with won't map to anyone. Really what we wanna see is if it makes the experience better for a noticeable amount of users. Cause then it might be worth merging it in (or at the very least making it an option).