Skip to content

Cache team and boss bar text conversions for older clients - #1380

Closed
bobhenl wants to merge 1 commit into
ViaVersion:masterfrom
bobhenl:cache-text-conversions
Closed

bobhenl wants to merge 1 commit into
ViaVersion:masterfrom
bobhenl:cache-text-conversions

Conversation

@bobhenl

@bobhenl bobhenl commented Oct 30, 2026

Copy link
Copy Markdown

Repeated team and boss bar texts were converted on every packet. The result is now cached, with a config switch to turn it off.

@kennytv

kennytv commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Repeated team and boss bar texts were converted on every packet

Do you have a spark report that shows this is a genuine issue? besides, given both structues are mutable, hashing and copying them full isn't exactly cheap either

@bobhenl

bobhenl commented Oct 30, 2026

Copy link
Copy Markdown
Author

Yes, allocation profiles from our server (26.3, 48–75 players, a lot of them on older clients):
Before: https://spark.lucko.me/KlIvpPsQnT – the BOSS_EVENT handler in Protocol1_20_3To1_20_2 (convertComponent/tagToJson) allocates 22.8 MB/s, and ViaRewind’s 1.9 BOSS_EVENT jsonToLegacy another 14.1 MB/s. Together that’s about 13 % of all allocation on the server, with 48 players.
After: https://spark.lucko.me/0FUBMF9qKk and https://spark.lucko.me/0ljflFzWSg – the same handler is at 4.0–4.4 MB/s with 72–75 players, and ViaRewind is at ~0. These numbers already include the hashing and copying, since that happens inside the same handler.
The cause is plugins resending the same bossbar title every few ticks. CPU cost per packet is small, it’s mostly GC pressure. Fair point that hashing isn’t free – it only pays off because the text repeats so often. Happy to change how the cache is keyed or bounded if you’d prefer another approach.

@kennytv

kennytv commented Oct 30, 2026

Copy link
Copy Markdown
Member

It's definitely noticeable allocation, but your gc pauses look very healthy, and this isn't really the right place to address it, see Lenni0451/MCStructs#120

@kennytv kennytv closed this Oct 30, 2026
@bobhenl

bobhenl commented Oct 30, 2026

Copy link
Copy Markdown
Author

Thanks, that PR makes sense as the proper place. Out of curiosity, would a small cache for unchanged boss bar titles still be acceptable on top of that, or do you prefer to keep it conversion-only? We’ll compare alloc profiles once it’s in a Via build either way.

@kennytv

kennytv commented Oct 30, 2026

Copy link
Copy Markdown
Member

Doesn't really make sense here tbh, there's 1000 other places this kind of cache could be added to otherwise. beyond the pr linked above, you should check on your plugins to not keep spamming the same bossbar lol

@bobhenl

bobhenl commented Oct 30, 2026

Copy link
Copy Markdown
Author

Fair point – I traced it, it’s CMI plugin re-sending the same boss bar title on every update tick. I'll report there. Thanks for the MCStructs PR!

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