image-rs / jpeg-decoder

JPEG decoder written in Rust
Apache License 2.0
150 stars 87 forks source link

Restructure multithreading #230

Closed HeroicKatora closed 2 years ago

HeroicKatora commented 2 years ago

Redefine the Worker trait, which allows the chosen worker to create a new scope. This is relevant for a newly created (and installed) rayon worker.

The rayon thread pool is now local to the decoding step. This fixes an issue where improper task scheduling would deadlock decoding. It's not clear how the intended task scheduling can be reliably achieved without the guarantee of having at least a second, free, worker thread.

Closes: #227

HeroicKatora commented 2 years ago

Added more tests, no based off the global pool, to demonstrate/test that this does not make a quantifiable difference. As suggested to add them here. @awused Did I miss any, do you have more suggestions?

HeroicKatora commented 2 years ago

Okay, scratch that. This is why it was a draft. How about this: In this rayon case, we always in_place_scope the main decoder loop. (The architecture should already be there after the rework). Then, instead of creating separate, parallel worker tasks, in append_row we run the Immediate worker as a new scoped task—this is already guaranteed non-blocking, still single threaded, but I'll get to that. Both start and get_result will run directly on the immediate worker without any of dubious communication structure of the multithreaded module.

Now the critical part: We modify Worker::append_row such that multiple rows can be spawned at the same time. The rayon implementation can then create multiple tasks in its scope, which lets other threads worksteal from us. Not sure about the runtime implications of this scheme. However, this way making the rayon worker entirely non-blocking might be easier than imagined.

HeroicKatora commented 2 years ago

I've given rayon usage a complete overhaul. No more blocking at all but Mutex to modify the shared result sets.