Skip to content

Implement a custom Int2Int map for MapPalette - #1414

Merged
booky10 merged 6 commits into
retrooper:2.0from
Vrganj:compact-map-palette
Apr 14, 2026
Merged

booky10 merged 6 commits into
retrooper:2.0from
Vrganj:compact-map-palette

Conversation

@Vrganj

@Vrganj Vrganj commented Dec 8, 2025

Copy link
Copy Markdown
Contributor

Replaces a very unnecessary HashMap<Object, Integer> with a much lighter custom Int2Int map.
Works fine from my tests, might warrant a unit test.

Closes #1412

Copilot AI review requested due to automatic review settings December 8, 2025 20:43

This comment was marked as low quality.

@Vrganj

Vrganj commented Dec 9, 2025 •

Copy link
Copy Markdown
Contributor Author

Did a rough comparison with GrimAC which stores all the chunks sent to players. The heap dump I took before the patch shows 33% of the total chunk memory data was used by palettes. After the patch, the usage dropped to about 17%, resulting in about a 20% memory improvement per average chunk in my case.

I'm aware they use a fork of packetevents so I applied the patch to their fork to test. Considering they seem to keep upstream commits up to date, I thought it'd be better to just merge the change here.

@booky10 booky10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please refactor this new Map into a separate class (and mark it as @ApiStatus.Internal then)

Also, you mentioned copying existing libraries in your issue - was this copied from somewhere or did you write this on your own?

@Vrganj

Vrganj commented Dec 21, 2025

Copy link
Copy Markdown
Contributor Author

Mostly wrote it myself, but took great inspiration from https://github.com/aeron-io/agrona/blob/master/agrona/src/main/java/org/agrona/collections/Int2IntHashMap.java which should be compliant licensing-wise. The map should probably be renamed to something like UInt2UIntMap considering I chose to use -1 as the empty key value.

@Vrganj
Vrganj requested a review from booky10 December 23, 2025 20:32
Stores key and value in one long to reduce array reads/writes
Also improves resizing logic by removing unnecessary replacement logic check

(Additionally also includes some minor code format changes)
@booky10
booky10 merged commit bbeec89 into retrooper:2.0 Apr 14, 2026
2 checks passed
@booky10

booky10 commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR and sorry that it took so long to merge!

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.

Memory consideration

3 participants