ElunaLuaEngine / Eluna

Eluna Lua Engine © for WoW Emulators
https://elunaluaengine.github.io
GNU General Public License v3.0
372 stars 356 forks source link

Asynchronous script (re)loading #487

Closed Foereaper closed 4 days ago

Foereaper commented 2 months ago

Currently we load scripts and precompile them in a temporary state on server startup and save them to a cache in the ElunaLoader. This is fine in and of itself, as it is done before the core has initialized properly, and it ensures that Eluna is ready for the startup hooks etc.

However, there's currently two "issues" with the reloading scheme during runtime.

First of all we call the reloading of the ElunaLoader script cache from within the global Eluna state itself. I don't think this is ideal. It works fine only because the command hook is called in the global state, but it is necessary with the way we currently handle the reload command (no command script but handled through the command hook).

Secondly, since this is ran synchronously, we freeze up the world thread with file IO, state creation and compilation. This is only a few milliseconds but I think this can be removed entirely.

I propose implementing some form of async threading scheme for the script loading and compilation, with an internal state to get the current state of the cache (none, reloading, ok, etc.). Since the state and the cache itself is never written to outside of the loader, it is safe to treat these as read-only. We would then flag the Eluna states for reloading like we already do, but in addition check the state of the script cache. If both reload is flagged and the cache state is OK, we proceed with the actual Eluna state reloading. Otherwise the state just keeps doing its thing until the cache is OK and we reset the reload flag.

We should also look into moving the reload functionality to the core itself and not handle it through some arbitrary Eluna state, especially since we are iterating over all the existing Lua states in that code.

Thoughts?

Foereaper commented 1 month ago

Cache reloading has been pushed, next thing to look at is moving the reload command out of the Eluna object, if that is something we want to look at.

Foereaper commented 4 days ago

After thinking about it, moving the command out gives us more core related stuff to manage. Not worth it.