Parallel script loading #3924
Parallel script loading #3924
Conversation
|
This PR seems a great idea and future for Skript to stop server lagging when reloading big scripts. I have read the config comments and I have a question, will this affect a minigame script for example? what could be the real disadvantages of reloading a script async? However, as I said, this is a great future for Skript. Thank you. |
There is no disadvantage of loading async or in parallel, except possible bugs and loading order issues, etc. It will affect any script obviously, say if one script is loading on 3 seconds on a 2 core CPU before, and with parallel loading configured to use 2 threads, it can load in 1.5 seconds, or a time like that. It will improve when there are more cores and threads. Though the server lag when reloading is already fixed by async loading, not parallel loading. Async loading loads on a different thread than the server thread so scripts load on background and not block the server from running. The 'lag' there is just freezes, basically the server will stop responding (ticking) until the reload is complete, but with async loading the server keeps running. However async loading will still load on one thread so the total time to load the scripts will not change, i.e, the script will still take 3 seconds to load but the server will continue to tick while it is loading. It can also increase load time however since both the server and the loading task needs to be run at the same time compared to only loading. Parallel loading uses more threads than one thread while also running the server, so it can make total load time faster and still not block the server from running like async loading. |
I agree with that, it would give more freedom to users to make scripts take less time to load if they have enough resources.
Regardless of ordering issues which is not my goal, since server will continue ticking and not freeze while for example a minigame script is reloading in the background, will the current game get affected in an unexpected way? such as losing track of players for sometime due to clearing script events/triggers or code in general while reloading? |
|
The current implementation of async (or parallel) loading is that the script is not unloaded until the new version of the script is fully loaded. Do note that the current implementation of parallel loading parallel on scripts, not within scripts, meaning that reloading a single script will take the same time with async loading as it does with parallel loading, you will only see the difference if you're reloading multiple scripts at once. |
Great to know that. This is indeed incredible accomplishment. |
…signature changed because of the OpenCloseables
Description
This PR adds parallel script loading. This PR also includes the following PRs (or a modified version of those PRs):
Parallel script loading can be enabled with a config options, see config.sk#L189.
Most fields in ScriptLoader related to script loading have been moved to ParserInstance, which uses a ThreadLocal to make parallel parsing possible, the same approach I took with #3800.
A lot of the script loading-related methods in ScriptLoader have been deprecated, and replaced by new methods that use CompletableFutures instead of directly returning values.
A part of the script loading process relied on public static fields, so these have been removed and replaced with (deprecated)
static getters and setters in ScriptLoader. Some addons are using these fields, so these will need to be updated, otherwise using their syntax elements (that use the removed fields) will throw a NoSuchFieldError. Because some addons are no longer maintained (or have slow development), I've created a tool that patches addons bytecode to use the getters and setters instead of the fields: TPGamesNL/SkriptAddonPatcher.
I could also keep the fields, but not use them, which only removes the NoSuchFieldError, instead breaking silently (sort of) (I would like feedback on this part).
Addons that use removed fields and need updating (only addons from SkriptTools.net)
Because this PR makes log handlers dependant on threads, log handlers should be passed as a parameter to the new script loading methods instead of being started/stopped before and after the loading method call. I've created the interface OpenCloseable for this (implemented by LogHandler), which has its open method called before a script is loaded, and it's close method after a script is loaded. This approach ensures that the log handler is always active during the script loading, but it may be opened and closed multiple times.
To get the ParserInstance for the current thread, the method
ParserInstance.get()should be used, but some classes (including SyntaxElement, therefore all syntax classes, effects, conditions, etc) have a shorthandgetParser()added, so they can use that method instead of the longerParserInstance.get().I've also added a data system to ParserInstance, which can be used by addons if they want to add their own data to the loading process (e.g. skript-reflect imports). These data classes can listen to changes to current script and changes to current events.
Target Minecraft Versions: any
Requirements: none
Related Issues: #1002, #2945 (I think), #3453, #3743, #3784, #3800, #3804, #3812