buserror / simavr

simavr is a lean, mean and hackable AVR simulator for linux & OSX
GNU General Public License v3.0
1.56k stars 365 forks source link

Removing the use of threads from ssd1306demo #253

Open dxxb opened 7 years ago

dxxb commented 7 years ago

I personally try to avoid threads when not necessary and found the ssd1306demo (and others) use threads. I have a single threaded version of ssd1306demo which I can push as a pull request if there is an interest in it. Note that operating in a single thread simplifies GLUT keyboard handling because there is no need to worry about thread safety.

buserror commented 6 years ago

I also try not to use threads when not necessary -- in this case I'm not sure why they are used, but perhaps it was to have the simavr simulation running at 'full speed' without being blocked by event related stuff. Technically there shouldn't be a lot of interaction in there.

dxxb commented 6 years ago

I suspect threads are used because GLUT's mainloop takes over the main thread and most GLUT examples out there use threads. By registering a GLUT idle callback we can run simavr in the same thread as the GLUT mainloop. I can confirm that running in a single thread does not impact the execution speed of the demo. FYI I am also using the replacement for avr_callback_sleep_raw() outlined in https://github.com/buserror/simavr/issues/254

buserror commented 6 years ago

Ah so that's why it slowed down and became noticeable; because glut idle callback must have quite a bit more overhead...

dxxb commented 6 years ago

Invoking avr_run() inside the idle callback and returning would make the simulation incur the GLUT mainloop overhead for each iteration. What I do is run avr_run() in a loop inside the idle callback and yield back to GLUT ~24 times per second when rendering is due.

buserror commented 6 years ago

Actually I think that's about as bad as the other solution really. here you are blocking glut, event processing and rely on synchronous behavior, while opengl is actually completely asynchronous by definition. Seriously, the 'proper' way to make this system work is to use a thread with avr stuff, and a couple of fifo/notifications to talk to talk to glut asynchronously. It doesn't need mutexes and stuff. Blocking glut isn't a nice solution. Decoupling the UI from the 'engine' is the proper way of doing this sort of stuff...

dxxb commented 6 years ago

Actually I think that's about as bad as the other solution really.

Actually it's not as bad as the other solution: in practice it works really well despite the fact that it was intended as a way to eliminate threads from the demo. And it works so well that I set it as default main loop implementation at compile time for the board I am working on.

here you are blocking glut, event processing and rely on synchronous behavior,

Yes but GLUT isn't doing anything most of the time except calling its idle callback when registered, otherwise it sleeps (i.e. blocks). So instead of letting GLUT call simavr, simavr yields to GLUT when rendering or keyboard polling is due.

Seriously, the 'proper' way to make this system work is to use a thread with avr stuff, and a couple of fifo/notifications to talk to talk to glut asynchronously. It doesn't need mutexes and stuff. Blocking glut isn't a nice solution.

I have implemented the thread+fifo design as a compile time option for the board I am working on but the goal here was not to make a better GLUT demo but a simavr demo, which happens to use GLUT, without threads. IMO there is no way to come up with something nice while using GLUT.