Skip to content
This repository has been archived by the owner on Jun 19, 2021. It is now read-only.

New async nbt cache #347

Merged
merged 7 commits into from
Jan 25, 2021
Merged

New async nbt cache #347

merged 7 commits into from
Jan 25, 2021

Conversation

HookWoods
Copy link
Contributor

The goal of this pr is to reduce I/O operations from the main thread while saving player data and also to avoid too many I/O operations while reading NBT Player file by using a cache (Which start to delete the oldest data when there is too much player compared to the map size)
I don't know if the map implementation is the best method or using a LRU system would be better

Copy link
Contributor

@MrIvanPlays MrIvanPlays left a comment

Choose a reason for hiding this comment

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

yet again I have to make the patch to a level...

@Titaniumtown
Copy link
Collaborator

yet again I have to make the patch to a level...

what?

@MrIvanPlays
Copy link
Contributor

yet again I have to make the patch to a level...

what?

there are diff problems, missing // Yatopia comments, unmarked imports...
I thought hugo would watch me do it on his other 2 prs but looks like he hasn't learned so I'd have to make it yet again

@Titaniumtown
Copy link
Collaborator

Ah ok. I didn't understand what you meant lmao.

@MrIvanPlays MrIvanPlays added the enhancement New feature or request label Jan 21, 2021
Copy link
Collaborator

@Titaniumtown Titaniumtown left a comment

Choose a reason for hiding this comment

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

LGTM, just needs that info in the patch commits, and proper commits when you change a section of code (as @MrIvanPlays said). But either than that, since it's been in a production environment, I see no reason as to why it shouldn't get merged!

@Titaniumtown Titaniumtown changed the base branch from ver/1.16.5 to dev/ver/1.16.5 January 23, 2021 22:50
Copy link
Collaborator

@Titaniumtown Titaniumtown left a comment

Choose a reason for hiding this comment

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

Along with @HookWoods using this patch privately in production for a while, I've been using it for the past 2 days on my server without any issues whatsoever. Seems good to merge into staging/1.16.5

@Titaniumtown Titaniumtown added the Ready for Review This PR is ready get reviewed in order to get merged label Jan 24, 2021
Copy link
Contributor

@ishland ishland left a comment

Choose a reason for hiding this comment

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

Seems good to me but there are still some small issues.

patches/server/0069-New-nbt-cache.patch Outdated Show resolved Hide resolved
patches/server/0069-New-nbt-cache.patch Outdated Show resolved Hide resolved
patches/server/0069-New-nbt-cache.patch Outdated Show resolved Hide resolved
patches/server/0069-New-nbt-cache.patch Outdated Show resolved Hide resolved
Base automatically changed from dev/ver/1.16.5 to staging/1.16.5 January 25, 2021 16:09
Copy link
Collaborator

@Titaniumtown Titaniumtown left a comment

Choose a reason for hiding this comment

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

LGTM. I addressed @ishland and my own requests.

@Titaniumtown Titaniumtown merged commit 7073c23 into staging/1.16.5 Jan 25, 2021
@Titaniumtown Titaniumtown deleted the dev/CacheNBT branch January 25, 2021 16:37
ishland added a commit that referenced this pull request Feb 1, 2021
* add config for sand duping (#352)

* Lithium: cache chunk gen sea level (#349)

based off: CaffeineMC/lithium-fabric@a55cfd1

* PaperPR: Inline shift fields in EnumDirection (#350)

* Introducing: Yatoclip (#360)

* New async nbt cache (#347)

* update pom

* whoops

* Try to address path issue and improve Jenkins build speed

* Detailed lag and crash reports (#369)

Added "Suspected Plugins" to Watchdog and crash reports

* Drop sand duping

* Add branch specific versions

* Remove copyright

* Revert mysql-connector-java version bump

* Small fixes

* More detailed lag and crash reports

* Don't use branch information when generating metadata

* Fix Jenkins Builds version command

* Fixup patches

* Fix patch notes

* Pull Request compatibility for branch detection

* Fix Pull Request compatibility for branch detection

* Set context classloader before launch

* Inject server jar to SystemClassLoader before launch

* Try fix compile in java8

* Run tests on CodeMC and Github Actions

Co-authored-by: Simon Gardling <Titaniumtown@gmail.com>
Co-authored-by: Zoe <duplexsys@protonmail.com>
Co-authored-by: Hugo Planque <12386279+HookWoods@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request Ready for Review This PR is ready get reviewed in order to get merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants