Thread issue
emperor06 opened this issue ยท 2 comments
May be linked to issue #42
Tick event uses a thread to refresh the block list while pickupItem and placeItem don't.
As a result, it is very likely that running around while destroying blocks calls blockFinder() twice in parallel.
The method safety comes for a delay (nextTimeMs) but the variable is updated at the end of the method so it is very possible for 2 threads (the main thread + the tick event thread) to enter the method at the same time, causing massive cpu load (#33 ?). Adding a delay on the thread just made it a bit less likely but it still occurs.
One idea would be to set blockFinder() as synchronized method but this will just block the main thread for several ticks, which is very bad.
I'm working on a revamp for the threads. Basically, here's what I've got so far:
- Forbid direct calls to blockFinder(). All calls must go through a manager that is responsible for avoiding multiple entries to the method and handling threads
- Using an singleThreadExecutor
- Make sure calls to blockFinder are processed unless it's already being processed
- discard multiple calls to blockFinder instead of queuing them
- remove the delay between calls: the mod can now update the block list to display whenever it can (so now moving around is a smoother experience as the display is updated pretty much in real time)
It's still a work in progress, there's a lot of testings to do but so far it seems to work like a charm. I need to optimize a couple of things though but please, feel free to take a look, test, and comment.
I've jumped over to your repo and I've been looking at what you've been chaning, I can't fault anything so far so well done!. You're handling it the way I intended to do it in the first instance but never got round to doing the refactor due to a lack of knowledge about threading at the time.
I've know about the issue #42 for a while and my assumption was that it was an event issue where the event was firing either before or after the next scan happened and caused it not to update. I wasn't far off but I hadn't figured that it might be calling the thread twice! I should have been doing a better check for that kinda thing.
As for the actual issue #42 my idea is to store the modid:blockname in the config and then onload go and get the block id from that data and store it in the OreInfo object. Allowing me to keep the fast referencing with ID's but a more logical method of storing in the config.
On another note, do you have a discord or anything? I'd love to have a chat with you about the mod