
Threading conundrum
Chocohead opened this issue ยท 3 comments
I'm in the situation where I have potentially a significant number of runtime assets which are relatively slow to read which all need giving to RRP. With the way this is designed I would've thought the rrp_pre
entrypoint would work, yet its implementation is just a serial loop in another thread. Whilst there is nothing wrong with this, it leaves me with a choice of three scenarios:
- Use the
rrp_pre
entrypoint and block the thread until the initial reading (which is still on thread from a differentpreLaunch
entrypoint) is done, then add the resources.- This is obviously non-ideal as now other resource generators are being held up by me waiting on the main thread. They probably won't be held up for that long, but I don't know how long they might take either.
- Use the
preLaunch
entrypoint and add things asynchronously in my own thread- This is non-ideal as it won't hold up resource loading if it happens to be slow (as only
RRPPre.PRE_GEN_LOCK
can do that). Potentially this is not important as the resources may not be accessed until later, but I cannot guarantee this for them all nor have a way of knowing for a given resource if this is the case. - As an aside the non-thread safe methods in
RuntimeResourcePack
should probably warn of it given they are not backed by thread safe collections. It's all quite innocent inRRPPreGenEntrypoint
which suggests they are all safe enough to call which could make for some strange CMEs from the main thread.
- This is non-ideal as it won't hold up resource loading if it happens to be slow (as only
- Use the
preLaunch
entrypoint and add things synchronously- This is non-ideal as it will hold the whole game up whilst I add things. Avoids any problems with being too slow for resource loading I guess ;)
I have a feeling everything isn't designed with threading in mind (which is totally fine. it can be a pain to design for), but if speed is the goal to use this over Artifice it shouldn't be left taped on IMO. I have given it a little thought about possible solutions at least if you'd like some ideas:
- Move to using a thread pool to handle asynchronous additions (possibly
RuntimeResourcePack#EXECUTOR_SERVICE
)- This allows shutting down the pool when it is time for the resources to be done, preventing late submissions.
- This fixes the spin loop in
ReloadableResourceManagerImplMixin#registerRDP
which is a bit naughty.
- Stop using
EntrypointUtils
and instead parallelise theList
returned byFabricLoader#getEntrypoints
.- This avoids people shouting at you for using internal APIs and fixes the entrypoints blocking each other if some are slower than others
- Wrap everything up in
Future
s and allow mods to chain their resources into the loading process- The Mojang way it feels like, not that you should necessarily listen to them :P
- Probably just a messier way of having a thread pool by abstracting it all
- Potentially easier for a modder to use if the external abstraction is clear (not that you couldn't just abstract a thread pool well directly)
- Go completely wild and allow other mods to register their intentions to a
Phaser
and then use that as a form of self asserted thread pooling.- Not entirely necessary if there's already a thread pool, but allows greater flexibility for other mods to use their own (optionally threaded) implementations.
- Also means the threads don't have to start until the work for them is ready, or even at all.
- Does open the possibility for the game to hang if a mod fails to announce their completion re-arriving or deregistering, but you can force
Phaser
s tostunterminate if they're being slow/you want them to with a single method call.
There's probably an argument for an entirely threaded form of RuntimeResourcePack
, maybe even using Semaphores
to limit concurrent writes or just as a lazy way of de-threading calls (or a ReentrantReadWriteLock
which would work for that task too), but maybe there's a point of diminishing returns eventually where it doesn't quite need to be that complicated. All food for thought in making the generation faster at least.
using the same executor service won't work in the event a mod decides to wait on an async task for whatever reason. I should have used a lock, and for some reason didn't. I think rrp is in need a big upgrade though, since modders can't choose the priority of the resource pack, along with the inefficiencies of using String#format for templating, and then creating a byte array from that
I've rewritten the whole thing from scratch, will commit soon:tm: once wiki is done, I think the threading is much better this time