iberianpig / fusuma

Multitouch gestures with libinput driver on Linux
MIT License
3.58k stars 146 forks source link

don't recheck events in `select_from_last_begin` #296

Closed ces42 closed 11 months ago

ces42 commented 1 year ago

This addresses part of https://github.com/iberianpig/fusuma/issues/295 . It fixes the inefficiency in https://github.com/iberianpig/fusuma/blob/b68ab60335260dc32bfb3748c0bf65f9dc6d9dad/lib/fusuma/plugin/buffers/gesture_buffer.rb#L99-L107 .reverse.find has bad runtime because it creates a reversed copy of the list (in linear time) and then iterates over it. But of course one should just iterate backwards over the list.

Furthermore instead of searching the whole buffer at every call of select_from_last_begin we should just search all entries that have been added since the last call and if none of them are a "begin" we just return the same as last time. This is what the two new variables @mem_last_begin and @mem_checked are for. When I run fusuma, the "begin" event is always at index 0 because the buffer is cleared before the next gesture starts and there's always one more entry in @events than in the last call. So this optimization is very efficient.

Finally, if the last "begin" did happen at index 0 we can return self instead of returning an identical copy.

possible problems that this could cause: Things will break if

profiling I enabled the line profiler and ran master vs my fork while (3-finger) swiping from one end of my touchpad to the other end (separate swipes left, right, left, right, etc.) with time exe/fusuma

current master:

lib/fusuma/plugin/detectors/swipe_detector.rb                                                                                                                                 
               |  18          # @return [NilClass] if event is NOT detected
               |  19          def detect(buffers)
   3.4ms  2682 |  20            gesture_buffer = buffers.find { |b| b.type == BUFFER_TYPE }
 315.3ms   894 |  21              .select_from_last_begin
 462.7ms 302172 |  22              .select_by_events { |e| e.record.gesture == GESTURE_RECORD_TYPE }
               |  23  
 310.4ms   894 |  24            updating_events = gesture_buffer.updating_events
   0.4ms   894 |  25            return if updating_events.empty?

this fork:

lib/fusuma/plugin/detectors/swipe_detector.rb
               |  18          # @return [NilClass] if event is NOT detected
               |  19          def detect(buffers)
   3.2ms  2745 |  20            gesture_buffer = buffers.find { |b| b.type == BUFFER_TYPE }
   9.3ms   915 |  21              .select_from_last_begin
 654.5ms 449571 |  22              .select_by_events { |e| e.record.gesture == GESTURE_RECORD_TYPE }
               |  23  
 454.9ms   915 |  24            updating_events = gesture_buffer.updating_events
   0.5ms   915 |  25            return if updating_events.empty?
iberianpig commented 12 months ago

@ces42 I apologize for the delayed review of the pull request. Thank you for tuning the performance. I have made some comments.

ces42 commented 12 months ago

Hi! I'm traveling right now so I probably won't have time to look at this until next week. I have made some more changes (locally) and I split the ones here into three separate branches, maybe that will make reviewing easier.