pyapp-kit / psygnal

Python observer pattern (callback/event system). Modeled after Qt Signals & Slots (but independent of Qt)
https://psygnal.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
84 stars 13 forks source link

perf: Fixing performance of evented set #275

Closed Czaki closed 8 months ago

Czaki commented 8 months ago

In this PR I have added an option for the combiner function to consume the whole list of tuples at once.

codspeed-hq[bot] commented 8 months ago

CodSpeed Performance Report

Merging #275 will improve performances by 11.28%

Comparing Czaki:speedup_evented_set (0ab5fc5) with main (459828c)

Summary

⚡ 1 improvements ✅ 65 untouched benchmarks

Benchmarks breakdown

Benchmark main Czaki:speedup_evented_set Change
test_create_signal 141.1 µs 126.8 µs +11.28%
Czaki commented 8 months ago

When developing locally, pre-commit complains about line length, but I do not know how to break type declaration.

Also, mypy report problems with lack of Self (I see that on Ci, it does not happen).

I need to add type casting for reducer to satisfy mypy.

codecov[bot] commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (459828c) 100.00% compared to head (0ab5fc5) 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #275 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 23 23 Lines 1898 1906 +8 ========================================= + Hits 1898 1906 +8 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Czaki commented 8 months ago

(Should I add benchmarks to tests/test_bench)?

tlambert03 commented 8 months ago

(Should I add benchmarks to tests/test_bench)?

it's ok. the asv benchmarks are nice because they can run retrospectively (and show the speedup you gained here), whereas codspeed and test_bench can only start measuring going forward. I like codspeed in general, but the lack of retrospective monitoring is a bummer, so the one you added to asv is good enough