Skip to content

Small optimization to prevent blocking netty threads on UUID.randomUUID()#1781

Open
Beaness wants to merge 4 commits intoPaperMC:dev/3.0.0from
PvPRivals:fix/playerinfoblock
Open

Small optimization to prevent blocking netty threads on UUID.randomUUID()#1781
Beaness wants to merge 4 commits intoPaperMC:dev/3.0.0from
PvPRivals:fix/playerinfoblock

Conversation

@Beaness
Copy link
Copy Markdown

@Beaness Beaness commented Apr 24, 2026

At higher playercounts a lot of player tablist packets can be sent concurrently causing some small thread congestion (across all netty threads). This is a small optimization to prevent this congestion by just generating the uuids using threadlocal random.

@electronicboy
Copy link
Copy Markdown
Member

invalid UUIDs is generally a no-go in my book, using a non-secure random also introduces some risks around duplicate UUIDs being generated

@Beaness
Copy link
Copy Markdown
Author

Beaness commented Apr 24, 2026

invalid UUIDs is generally a no-go in my book, using a non-secure random also introduces some risks around duplicate UUIDs being generated

Changed to a valid uuid v4, I don't think the duplicate uuid is a real issue as there are so many random bits I don't think it will ever collide

@WouterGritter
Copy link
Copy Markdown
Contributor

WouterGritter commented Apr 24, 2026

Have you measured the performance of java's random UUID generation and this proprietary implementation? Is it even worth it to roll our own, aside from possible security/duplicate UUID concerns?

@Beaness
Copy link
Copy Markdown
Author

Beaness commented Apr 24, 2026

Have you measured the performance of java's random UUID generation and this proprietary implementation? Is it even worth it to roll our own, aside from possible security/duplicate UUID concerns?

Yes at higher player counts there is thread congestion on netty threads (this worsens with the higher the threads and all players getting the same player info packets at the same time)

The security UUID concern should not be applicable here as they are only used as identifiers internally.
I kept the UUID.randomUUID() in click callbacks to ensure the security.

@WouterGritter
Copy link
Copy Markdown
Contributor

WouterGritter commented Apr 24, 2026

Also, how "unique" do the UUIDs have to be in the tablist? Could we increment the lower or higher long to keep the UUIDs "locally" unique per session or per tablist? That would speed this up a lot. Kind of similar how anonymous player entries in the server player list packets are expected to have a uuid of new UUID(0, 0). Though that does abuse the hell out of the universally unique IDs here.

@Beaness
Copy link
Copy Markdown
Author

Beaness commented Apr 24, 2026

Also, how "unique" do the UUIDs have to be in the tablist? Could we increment the lower or higher long to keep the UUIDs "locally" unique per session or per tablist? That would speed this up a lot. Kind of similar how anonymous player entries in the server player list packets are expected to have a uuid of new UUID(0, 0). Though that does abuse the hell out of the universally unique IDs here.

This should technically also work for the legacy tablist implementations. The uuid is used to have a unique id for a player name as UUIDs aren't send in legacy with the tablist packets.

@WouterGritter
Copy link
Copy Markdown
Contributor

WouterGritter commented Apr 24, 2026

https://github.com/f4b6a3/uuid-creator

UUID Version 7: the Unix Epoch time-based UUID specified in RFC 9562.

Would be a perfect fit here (UuidCreator#getTimeOrderedWithRandom / UuidCreator#getTimeOrderedEpoch[Fast]).

@Beaness
Copy link
Copy Markdown
Author

Beaness commented Apr 24, 2026

https://github.com/f4b6a3/uuid-creator

UUID Version 7: the Unix Epoch time-based UUID specified in RFC 9562.

Would be a perfect fit here (UuidCreator#getTimeOrderedWithRandom / UuidCreator#getTimeOrderedEpoch[Fast]).

Is it really needed? The current implementation keeps the v4 validity and the probability of actually having a collision is astronomically low.

@WouterGritter
Copy link
Copy Markdown
Contributor

I think relying on an external library for this rather than manually fiddling with bits in our own FastRandomUuid is favorable. Its current name also suggests its perfectly safe to use in other cases where a random uuid is needed, which its really not due to relying on a non-secure random generator.

I think a better solution would be to get rid of this class and move it to VelocityTabListLegacy as a private static method (does the bossbar class actually re-generate the random uuids a bunch of time?)

@Beaness
Copy link
Copy Markdown
Author

Beaness commented Apr 24, 2026

I think relying on an external library for this rather than manually fiddling with bits in our own FastRandomUuid is favorable. Its current name also suggests its perfectly safe to use in other cases where a random uuid is needed, which its really not due to relying on a non-secure random generator.

I think a better solution would be to get rid of this class and move it to VelocityTabListLegacy as a private static method (does the bossbar class actually re-generate the random uuids a bunch of time?)

The method is now only present in VelocityTabListLegacy. I don't think an additional library is really needed, the collision chances are basically zero and the uuid is structurally valid. It's also not a usable "uuid" as it's used to spoof a tablist entry for temporary binding to a tab name.

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.

3 participants