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

refactor(CraftingManager): change packet fields to private and optimize packet caching #407

Merged
merged 3 commits into from
Feb 21, 2025

Conversation

lt-name
Copy link
Member

@lt-name lt-name commented Feb 20, 2025

No description provided.

…ze packet caching

refactor(Server): simplify recipe list sending logic using cached packets
@lt-name lt-name marked this pull request as ready for review February 20, 2025 12:34
@lt-name lt-name requested a review from a team February 21, 2025 03:51
@Mcayear
Copy link
Contributor

Mcayear commented Feb 21, 2025

The rebuildPacket function no longer builds packets for all versions, but only for the latest version (ProtocolInfo.CURRENT_PROTOCOL).

Positive:

This will optimize initial startup speed and memory usage.

Negative:

I hypothesize that a player not using ProtocolInfo.CURRENT_PROTOCOL might experience lag-induced disconnections upon their first login due to this change.


Overall, this is a viable performance optimization.

@Mcayear
Copy link
Contributor

Mcayear commented Feb 21, 2025

Start Speed:

Test Group Raw Data (seconds) Data after Removing Outliers (seconds) Average (seconds)
1 8.474, 8.378, 8.697, 9.465, 8.867, 9.417, 8.773, 8.63 8.474, 8.697, 8.867, 9.417, 8.773, 8.63 8.810
2 9.702, 9.683, 9.996, 9.254, 7.884 9.702, 9.683, 9.254 9.546

I have used "Data after Removing Outliers" which is a common and clear way to describe the data after the highest and lowest values have been removed.

RAM Usage:

Test Group Memory Data (MB) Average Memory Usage (MB)
1 308.58, 306.15 307.365
2 361.45, 378.81 370.13

Overall, the speed optimization is not significant, while the memory savings are approximately 60 MB.

@Mcayear
Copy link
Contributor

Mcayear commented Feb 21, 2025

@lt-name lt-name merged commit e617e14 into master Feb 21, 2025
1 check passed
@lt-name lt-name deleted the feat/improve-crafting branch February 21, 2025 09:09
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