-
Notifications
You must be signed in to change notification settings - Fork 88
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
Add Folia support #384
base: master
Are you sure you want to change the base?
Add Folia support #384
Conversation
any update on this? |
Thank you for your pull request. Unfortunately, I believe this will not suffice to make IF work correctly with Folia, for the following reasons. I think we should first consider how we want plugins that use IF to interact with it on Folia servers. Currently on Bukkit-based servers, all calls to IF code must be done from the main server thread. Any calls from a different thread give no guarantee that any method performs according to its contract. In return, any calls we make to your code (e.g., a click handler on I think there are several options that could work with regards to Folia support. The first one is to keep it like this, expecting all plugins to use a specific thread (GlobalRegionScheduler probably makes the most sense?) and in return all their code is also called on this thread. The advantage of this is that all the internal code for panes, guis, Another option is to rewrite everything so every single thing supports multiple threads. Replace every collection with an appropriate collection that supports multiple threads, replace fields with atomic variants, make sure all of the methods changes are done in a way where they seem atomic, etc. etc. etc. This is a lot of work, but would avoid the context switching needed when code from an arbitrary thread tries to interact with IF code. There may still be a context switch when IF code tries to interact with players as there's no guarantee that the code IF is currently running on is the region thread for the player, but compared to the first option this would only be a possibility, not a guarantee. Plugins that create their own panes would need to write their pane such that it supports multiple threads. Handlers can be called on any thread, so plugins that require a certain thread would still need to handle this, but if their code doesn't require a specific thread, no context switch is needed. In the end, we should get a performance boost from the reduced amount of context switching, probably with a decrease in performance on Bukkit-based servers, due to all the overhead from the code needed to make multiple threads play nicely with each other. Plugins could now also call methods from multiple threads on Bukkit, though, although the usefulness of this is probably pretty limited. I think it makes most sense to go with option one, since it's significantly easier. Given this, there are many changes that need to be made. Gui#update() AnvilGui#show(HumanEntity), BarrelGui#show(HumanEntity), BeaconGui#show(HumanEntity), BlastFurnaceGui#show(HumanEntity), BrewingStandGui#show(HumanEntity), CartographyTableGui#show(HumanEntity), ChestGui#show(HumanEntity), CraftingTableGui#show(HumanEntity), DispenserGui#show(HumanEntity), DropperGui#show(HumanEntity), EnchantingTableGui#show(HumanEntity), EnderChestGui#show(HumanEntity), FurnaceGui#show(HumanEntity), GrindstoneGui#show(HumanEntity), HopperGui#show(HumanEntity), MerchantGui#show(HumanEntity), ModernSmithingTableGui#show(HumanEntity), ShulkerBoxGui#show(HumanEntity), SmithingTableGui#show(HumanEntity), SmokerGui#show(HumanEntity), and StonecutterGui#show(HumanEntity) BarrelGui#show(HumanEntity), BeaconGui#show(HumanEntity), BlastFurnaceGui#show(HumanEntity), BrewingStandGui#show(HumanEntity), CartographyTableGui#show(HumanEntity), ChestGui#show(HumanEntity), CraftingTableGui#show(HumanEntity), DispenserGui#show(HumanEntity), DropperGui#show(HumanEntity), EnchantingTableGui#show(HumanEntity), EnderChestGui#show(HumanEntity), FurnaceGui#show(HumanEntity), HopperGui#show(HumanEntity), ShulkerBoxGui#show(HumanEntity), SmithingTableGui#show(HumanEntity), SmokerGui#show(HumanEntity), and StonecutterGui#show(HumanEntity) MerchantGui#addTrade(MerchantRecipe, int), and MerchantGui#addTrade(MerchantRecipe) GuiItem(ItemStack, Consumer, Plugin), GuiItem(ItemStack, Plugin). GuiItem(ItemStack, Consumer), GuiItem(ItemStack), and GuiItem(ItemStack, Consumer, Logger, NamespacedKey) GuiListener GuiListener#onEntityPickupItem(EntityPickupItemEvent) GuiListener#onPluginDisable(PluginDisableEvent) InventoryComponent#display(PlayerInventory, int) InventoryComponent#placeItems(PlayerInventory, int) HumanEntityCache#storeAndClear(HumanEntity) HumanEntityCache#store(HumanEntity) HumanEntityCache#restore(HumanEntity) NMS Please do note that we also want compatibility with Bukkit, so all this would need to be done such that it falls back to running normally if on Bukkit (similar to what you've done currently, but probably needs to be extended). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above comment
669082a
to
d4eced8
Compare
d4eced8
to
87bb8ac
Compare
Fixes #383.