Do not send duplicated wakeups to main loop - #1082

incrementing and comparing integers is not atomic, so you cant really use it for cross-thread synchronization. And while you could possibly use atomic operations for integer increment and comparison, wait_for_events() and wakeup_main_loop() themselves are not atomic. which means you could end up missing wakeups if the timing is not right

also as far as I know, wait_for_events() should wakeup just once even if there are multiple calls to wakeup_main_loop(). Is that not the case on macOS?

Each call to wakeup_main_loop() dispatches an event to main loop and wait_for_events() would need to handle all of them, which takes a long time (~0.5 seconds in macOS while running seq, not sure about linux).

Atomic integer is not required here since only one thread would alter the value, the other thread just reads.

There's indeed small chances of missing wakeups (when a wakeup is to be sent between wait_for_events() and main_loop_id += 1), but there's always timeout for waiting.

I would try to work out another solution without causing missing wakeups if I got some free time.

yeah but the event is an empty event, it does not actually require any handling, beyond reading it off the queue. I'm surprised that is expensive in cocoa. It might be possible to further optimize it in _glfwPlatformPollEvents in cocoa_window.m Check the event type and if it is NSEventTypeApplicationDefined dont call sendEvent with it.

I tried to not send out NSEventTypeApplicationDefined events in _glfwPlatformPollEvents, it doesn't help much.

Ok well I suppose we can use something along the lines of what you propose, the chances of delayed input processing are pretty low even without using mutexes, which might be expensive.

Even if only one thread is writing and one thread is reading, without using mutexes there will still be data races and thus undefined behaviour. See https://en.wikipedia.org/wiki/Race_condition#Software

Yes you're right. Memory fence should be used.

An alternate approach is to make use of the fact that the main thread only parses input if input_delay time has passed. So there is no need for the IO loop to wakeup the main loop more often than that. I dont however know if the input delay is large enough to materially affect performance or not.

Does this commit increases maximum input delay to be input_delay * 2?

Edit: more than that. Given input_delay=100, if first poll call takes 99 ms and second poll call takes forever, the wakeup would never be triggered?

poll now takes a timeout. So the second poll call can take at most 1ms in your example. and why would maximum input delay double? just as poll now takes a timeout in the I/O loop, the main loop check for input delay also takes a timeout (sets maximum wait time).

Ah, my bad, I though both IO loop and main loop (parse function) would wait for input_delay, I was wrong, they use new_input_at timestamp.

👍

For the poll timeout, I would suggest using ceil(time_delta) to prevent unnecessary multiple timeouts.

the poll timeout is wrong anyway right now, the value needs to be converted from seconds to milliseconds

fixed and also used ceil

Oh and not to mention that in your scenario above the main_loop would set max_wait_time to 1 ms and wakeup itself 1ms later