feat: alter clipboard thread to move all clipboard loading from main thread #2867
Conversation
|
Currently untested |
worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/WorldEditPlugin.java
Outdated
Show resolved
Hide resolved
worldedit-core/src/main/java/com/fastasyncworldedit/core/Fawe.java
Outdated
Show resolved
Hide resolved
worldedit-core/src/main/java/com/sk89q/worldedit/extension/platform/AbstractPlayerActor.java
Outdated
Show resolved
Hide resolved
dordsor21
force-pushed
the
feat/alter-clipboard-thread-usage
branch
3 times, most recently
from
7ab49c8 to
e1bd063
Compare
worldedit-core/src/main/java/com/fastasyncworldedit/core/Fawe.java
Outdated
Show resolved
Hide resolved
dordsor21
force-pushed
the
feat/alter-clipboard-thread-usage
branch
from
e1bd063 to
fc049d6
Compare
...dit-core/src/main/java/com/fastasyncworldedit/core/util/task/UUIDKeyQueuedThreadFactory.java
Show resolved
Hide resolved
| Settings.settings().PATHS.CLIPBOARD + File.separator + getUniqueId() + ".bd" | ||
| ); | ||
| try { | ||
| Future<?> fut = Fawe.instance().submitUUIDKeyQueuedTask(getUniqueId(), () -> { |
There was a problem hiding this comment.
Is it really wanted to enqueue twice? LocalSession#loadCLipboardFromDisk(File) submits a task in the KeyQueuedExecutorService - or is the threading purely for acquiring / synchronizing on the lock?
There was a problem hiding this comment.
That's done on purpose as we want to ensure that we offload from the main thread in AbstractPlayerActor, but without creating a new thread, or using up a thread in an unrelated thread pool. Since we will end up resubmitting to the queue pool again (which results in immediately running the task) we don't lose anything, but only gain safety in offloading in two places
worldedit-core/src/main/java/com/sk89q/worldedit/extension/platform/AbstractPlayerActor.java
Outdated
Show resolved
Hide resolved
| @@ -423,7 +423,7 @@ default void unregister() { | |||
| if (Settings.settings().CLIPBOARD.USE_DISK && Settings.settings().CLIPBOARD.DELETE_ON_LOGOUT) { | |||
| session.deleteClipboardOnDisk(); | |||
| } else if (Settings.settings().CLIPBOARD.USE_DISK) { | |||
| Fawe.instance().getClipboardExecutor().submit(getUniqueId(), () -> session.setClipboard(null)); | |||
| Fawe.instance().submitUUIDKeyQueuedTask(getUniqueId(), () -> session.setClipboard(null)); | |||
There was a problem hiding this comment.
Kinda the same as my first comment, #setClipboard has basically no work except for synchronizing on the lock - is enqueuing that worth it? Might be required due to the lock holder thread, but that would require getClipboard calls to be enqueued as well, which they aren't.
There was a problem hiding this comment.
Yeah so basically "free" synchronization from the main thread without having to wait for any potentially ongoing clipboard loading (players joining and leaving a server instantaneously could otherwise lag the server)
No description provided.