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

EventedSet.update is surprising slow #271

Closed Czaki closed 8 months ago

Czaki commented 8 months ago

Description

The original performance problem is here: https://github.com/napari/napari/issues/6275#issuecomment-1939599691

I have created a simple script to find problem

from psygnal.containers import EventedSet

my_set = EventedSet()
my_set.events.items_changed.connect(print)
normal_set = set()

num = 1_000_00

my_set.update(range(num))
normal_set.update(range(num))

And profiling points here:

obraz

https://github.com/pyapp-kit/psygnal/blob/850d02d33d6cafac86447872c389f479652e9501/src/psygnal/containers/_evented_set.py#L305-L309

When I change to list extend it is much faster (I have updated also initial value to ([], []):

def _reduce_events(a: tuple, b: tuple) -> tuple[tuple, tuple]:
    """Combine two events (a and b) each of which contain (added, removed)."""
    a0, a1 = a
    b0, b1 = b
    a0.extend(b0), a1.extend(b1)
    return (a0, a1)

obraz

It looks like the current approach is not the best one, and it will be worth to allow providing reducer function that could consume the whole event queue without using functols.reduce

It may be worth to also adding optimization with checking if any callback is connected to the event. And If not, then just drop it.

Czaki commented 8 months ago

I understand your reaction ass suggestion to make a PR?

tlambert03 commented 8 months ago

If you have a suggestion, then PRs are always appreciated! If you don't, then I'll have a look and see what can be done when I have time

Czaki commented 8 months ago

closed in #275