introduce optional futures support + further refactoring

fogti commented 4 years ago

Because the event API is generally asynchronous, it should be a perfect fit for the API to provide the possibility to wrap an EventQueue into something which implements Stream (from the futures API), which would allow easier usage in (partially) asynchronous multi-threaded code.

It should also make it easier to build something like the cascade API entirely in user code, with the futures_util::select! macro, with probably better ergonomics.

TODOs:

[X] Fix schans code to provide thread-safe Listeners
[X] Check if pinning projections might be appropriate -> (currently) not appropriate
[X] Avoid clones, possible using guarded references (like RefCell::borrow)
[X] Fix performance bottlenecks (probably negligible)
[X] Refactor thread-safe API to avoid code repetition
[X] Try to minimize changes (got rid of some unendearing API parts, hacks and probably useless restrictions; includes some additions, but is probably balanced)
[X] Re-introduce new -> default forwarding and some wrapper types for the non-TS cases
[X] Minimize trait bounds if possible (postponed)

Done:

I renamed event queue push -> emit* to avoid confusion when working with stacked event queues based on Vec.
I introduced EmitResult as return value of emit*

Related issues:

https://github.com/rust-windowing/winit/issues/1292

fogti commented 4 years ago

P.S. I noticed that the tests (incl. doc tests) for the main reclutch crate are failing, but I don't know how to really fix them.

jazzfool commented 4 years ago

The actual tests pass, it seems to be because of the library-level documentation/introduction.

fogti commented 4 years ago

another interesting thing would be to refactor the event API to allow plugging different extensions into thread-safe event queues (chans, dchans, schans, ...).

fogti commented 4 years ago

IMPORTANT: I'll probably squash this soon. It shouldn't be merged as-is. But it is ready for review.

fogti commented 4 years ago

I found some performance regressions:

rcevent-listener-with   time:   [311.17 ns 315.92 ns 321.24 ns]                                  
                        change: [-7.0552% -5.0255% -3.0932%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  9 (9.00%) high mild
  1 (1.00%) high severe

nonrcevent-listener-peek                                                                            
                        time:   [341.74 ns 350.75 ns 360.34 ns]
                        change: [+3.8808% +6.3163% +8.5222%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild

nonrcevent-listener-with                                                                            
                        time:   [263.31 ns 267.59 ns 272.58 ns]
                        change: [-9.3239% -7.4884% -5.6072%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  5 (5.00%) high mild
  3 (3.00%) high severe

nonrcevent-cleanup      time:   [841.71 ns 849.13 ns 858.64 ns]                                
                        change: [+17.377% +18.871% +20.353%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe

rawevent-pull-with      time:   [626.92 ns 628.76 ns 630.97 ns]                                
                        change: [-4.4102% -3.0745% -1.6697%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 12 outliers among 100 measurements (12.00%)
  4 (4.00%) high mild
  8 (8.00%) high severe

event-merge-map         time:   [766.55 ns 773.24 ns 780.13 ns]                             
                        change: [-6.9993% -5.2339% -3.4589%] (p = 0.00 < 0.05)
                        Performance has improved.

fogti commented 4 years ago

What's out of scope of this PR, but might be a good idea to investigate, is the optional addition of calloop support in the events API.

fogti commented 4 years ago

ok, squashed. ready to merge if tests pass.

jazzfool commented 4 years ago

Merged, thanks!

fogti commented 4 years ago

@jazzfool could you please remove the 'WIP' tag?

jazzfool / reclutch

introduce optional futures support + further refactoring #13