World multithreading #727

Janmm14 · 2021-01-28T17:15:47Z

Closes #715
Closes #728
Closes #751

Documentation PR: gomint/gomint-docs#28

I'd like to get some hints on what else needs to be made multithread-ready.

TODO:

World events (join, leave, load, unload)
Command API
Console commands
permission ticking
Easy, standardized API for plugins to read world allow-/blocklist out of config
Scoreboards ?!
world-limited event listeners
External documentation

Current design changes to #715:

Only one network manager, but it only ticks worldless connections, otherwise the world ticks the connections
New connections are first handled on the old main thread until player joined a world
Chat stays on each world's thread, I think that's better than an extra chat thread.

Also contains a bunch of various fixes as well as some javadoc and code layout fixes.

geNAZt

This system should be made with more generic prediction usage. So you can have registerLisetener(T listener, Predicate<Event> shouldFirePredicate) and there is no need to store the IntSet (which is a candidate for hash collisions) since you can use the Collection (which should be moved to the Set interface) in SimplePluginManager eventManager.registerListener(EventListener, Collection<String>) to

eventManager.registerListener(EventListener, Predicate<Event>)

eventManager.registerListener(EventListener, new Predicate<Event>() {
  boolean test(Event event) {
    return !(event instanceof WorldEvent && worlds != null && !worlds.contains(((WorldEvent) event).world().folder()));
  }
})

geNAZt · 2021-01-29T08:40:01Z

Can you also decouple the API / this reference cleanup from the actual world multithreading? Its hard to review in the current state due to that

geNAZt · 2021-01-29T09:44:17Z

This system should be made with more generic prediction usage. So you can have registerLisetener(T listener, Predicate<Event> shouldFirePredicate) and there is no need to store the IntSet (which is a candidate for hash collisions) since you can use the Collection (which should be moved to the Set interface) in SimplePluginManager eventManager.registerListener(EventListener, Collection<String>) to
eventManager.registerListener(EventListener, Predicate<Event>)

eventManager.registerListener(EventListener, new Predicate<Event>() {
  boolean test(Event event) {
    return !(event instanceof WorldEvent && worlds != null && !worlds.contains(((WorldEvent) event).world().folder()));
  }
})
I used the IntSet, because the WorldManager currently does the same.

Can you link me that usage? If there is a IntSet in use in that Manager its for sure a bug and should be fixed ASAP

geNAZt · 2021-01-29T10:01:43Z

Yes, can you open a PR with that here?

- split sync and async scheduler api and implementation - add per-world sync scheduler - move ticking code from GoMintServer to each world - create thread in world constructor - add plugin instance field to plugin class loader and getCallerPluginLoader() method in CallerDetector for world#scheduler() method returning plugin-based - replace main thread check with world thread check - synchronize access to Entity#world - schedule connection close handling onto world thread

- remove sync scheduling from PacketLoginHandler as disconnect is thread safe now - remove constructor arguments from PacketHandlers - update world of player to spawnLocation world of PlayerLoginEvent - make world manager thread safe, start world thread

…prove concurrency in NetworkManager.

…ld thread

…WeakReference, clear syncTaskManager on world unload

…rrentTickTime where used

gomint-server/src/main/java/io/gomint/server/world/WorldAdapter.java

…-multithreading

…or global commands, also add option to disallow console.

…for command map sending

- Move server.executorService() to server.asyncScheduler() to be notified about exceptions happening - move player to world tick earlier, add newConnections list to each world for that - fix npe in login as spawnLocation can be null - network manager untick queue is now just an arraylist (enough) - Change closing behaviour: - close network later to be able to kick players - disallow new connections when shutting down - players get kicked with "Server closed" instead of just connection closing. - fix potential watchdog npe when thread is dead - report exceptions in world tick and shutdown server - uncaught exception handler reports thread id as well - added a couple more debug log messages

Janmm14 · 2021-02-15T12:09:05Z

I think it might be now ready to merge.
Did some testing and found no more obvious bugs.
Not sure if you want more documentation on certain topics.

Janmm14 · 2021-02-15T16:36:04Z

A topic to look at is whether we need more safe world access checks, but I think that could be done after merging.

gomint-api/src/main/java/io/gomint/GoMint.java

gomint-api/src/main/java/io/gomint/command/CommandOutput.java

gomint-api/src/main/java/io/gomint/event/entity/projectile/ProjectileHitBlocksEvent.java

gomint-api/src/main/java/io/gomint/plugin/Plugin.java

gomint-server/src/main/java/io/gomint/server/util/Precondition.java

gomint-server/src/main/java/io/gomint/server/world/WorldManager.java

gomint-server/src/main/java/io/gomint/server/world/WorldAdapter.java

- Remove Command scope system again, Commands are now always bound to active worlds. - Event listeners are now always bound to active worlds. - Removed some more unneeded calls to CommandOutput#markFinished

…atchCommand

… stop.

Janmm14 force-pushed the world-multithreading branch 2 times, most recently from 011c48c to 7502658 Compare January 28, 2021 18:23

geNAZt requested changes Jan 29, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

Janmm14 force-pushed the world-multithreading branch from 1e3470a to 1bef9e7 Compare January 29, 2021 09:49

Janmm14 mentioned this pull request Jan 29, 2021

World multithreading Janmm14/GoMint#2

Closed

This comment has been minimized.

Sign in to view

Janmm14 force-pushed the world-multithreading branch from b97fe66 to 01c6e98 Compare January 29, 2021 14:25

Janmm14 mentioned this pull request Jan 29, 2021

Various refactor #729

Merged

Janmm14 added 16 commits January 29, 2021 17:43

Move to controlled network manager handoff for player connections. Im…

77936bc

…prove concurrency in NetworkManager.

EntityPlayer imports

9e39d2f

World switching now does schedule the target world part in target wor…

55340af

…ld thread

more multithread warnings, PluginWorldSchedulerAdapter stores only a …

b7909dd

…WeakReference, clear syncTaskManager on world unload

Force waiting in WorldManager#close until all worlds are stopped.

0d73dcf

Remove static WorldAdapter logger

a099217

Stop asyncWorkerTask on world close

4dc9463

Don't complain about async world worker accessing world data async

e4621a0

Fix world threadId init

4f67d32

Fix tests

26651fd

Outsource event caller class creator code to static helper method

8414f06

module-info: Export io.gomint.event.interfaces

d0d8a28

Add world-whitelisted event listeners

684fed1

Remove server tps and server currentTickTime and switch to world's cu…

82c6f8c

…rrentTickTime where used

Janmm14 mentioned this pull request Feb 13, 2021

Documenting event system and world multithreading. General improvements. gomint/gomint-docs#28

Open

This comment has been minimized.

Sign in to view

Merge branch 'master' into world-multithreading

c01fbef

geNAZt reviewed Feb 14, 2021

View reviewed changes

gomint-server/src/main/java/io/gomint/server/world/WorldAdapter.java Outdated Show resolved Hide resolved

Janmm14 marked this pull request as draft February 14, 2021 11:59

Janmm14 added 11 commits February 14, 2021 13:21

Add activeWorldsPlayers API

69734fe

Merge remote-tracking branch 'origin/world-multithreading' into world…

14b3ae6

…-multithreading

Commands are restricted to plugin's active worlds by default. Allow f…

297cd47

…or global commands, also add option to disallow console.

Fluent getters for Command, SubCommand now checks active worlds also …

b50abd0

…for command map sending

Add UncaughtExceptionHandler to threads.

54e1cfd

Let environment variable LOG_LEVEL define log level

e42c601

Revert @Version javadoc tag changes

8e75423

Remove unneeded markFinished from TestCommand

e80db13

ignore files of running gomint in release folder

1830b3b

@author Janmm14

22d182b

Janmm14 marked this pull request as ready for review February 15, 2021 12:08

Janmm14 requested a review from geNAZt February 15, 2021 14:17

Add toString with super class accessors to events. Fixes gomint#719

cbcfd86

geNAZt requested changes Feb 28, 2021

View reviewed changes

Janmm14 added 2 commits February 28, 2021 17:28

Address some requested changes, add isRunning to server & worlds api

237361d

Remove unneeded markFinished call

cd561b6

geNAZt reviewed Feb 28, 2021

View reviewed changes

gomint-server/src/main/java/io/gomint/server/world/WorldAdapter.java Show resolved Hide resolved

Janmm14 added 4 commits March 1, 2021 15:35

Address more requested changes.

2f89294

- Remove Command scope system again, Commands are now always bound to active worlds. - Event listeners are now always bound to active worlds. - Removed some more unneeded calls to CommandOutput#markFinished

Update javadoc in CommandManager to reflect current behaviour of disp…

98dd3eb

…atchCommand

Don't shutdown server on world crash. Kick remaining players on world…

de219dd

… stop.

Code style: Remove an unneeded cast

98e99f4

Janmm14 requested a review from geNAZt March 9, 2021 15:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

World multithreading #727

World multithreading #727

Janmm14 commented Jan 28, 2021 •

edited

Loading

geNAZt left a comment

geNAZt commented Jan 29, 2021

This comment has been minimized.

geNAZt commented Jan 29, 2021

This comment has been minimized.

This comment has been minimized.

geNAZt commented Jan 29, 2021

This comment has been minimized.

This comment has been minimized.

Janmm14 commented Feb 15, 2021 •

edited

Loading

Janmm14 commented Feb 15, 2021

World multithreading #727

Are you sure you want to change the base?

World multithreading #727

Conversation

Janmm14 commented Jan 28, 2021 • edited Loading

TODO:

Current design changes to #715:

geNAZt left a comment

Choose a reason for hiding this comment

geNAZt commented Jan 29, 2021

This comment has been minimized.

geNAZt commented Jan 29, 2021

This comment has been minimized.

This comment has been minimized.

geNAZt commented Jan 29, 2021

This comment has been minimized.

This comment has been minimized.

Janmm14 commented Feb 15, 2021 • edited Loading

Janmm14 commented Feb 15, 2021

Janmm14 commented Jan 28, 2021 •

edited

Loading

Janmm14 commented Feb 15, 2021 •

edited

Loading