Signal performance improvements
Jeremy Dunck
Feature tradeoffs:
Strong/weak receiver connections - strong-only is a big win-- about 2x
performance. Any receivers going out of scope (and not to be called)
should be explicitly disconnected if we remove support for weak
receivers.
.send API enforcement - about a 20% overhead to ensure signals are
sent properly. Worth it?
API whittling for extensibility: Leo has correctly pointed out that
**kwargs can be used for the same purpose. Functions that accept
kwargs are fast-pathed, but maybe we should change to passing all
receiver arguments with kwargs to avoid the overhead entirely?
kwargs vs whittling: 20% overhead to match APIs
In general, the latest patch optimizes for .send at the expense of
extra overhead in .connect and .disconnect. .connect and .disconnect
are O(n) on number of active receivers for that signal; I don't
anticipate that being a big problem, though.
One other enhancement I thought might be good is to have a
Model.pre_init signal, similar to Model.DoesNotExist so that
.connect(class=Model, ...) would allow listening to, say, pre_init
only for instantiation of that model. Other similar class-based
receiver lists might speed things up in some cases; the down-side is
that each receiver list has some overhead, even if empty.
Please see the ticket for timings and alternate implementations.
Regardless of the outcome, the patch also adds regression tests for
signals which I believe would be useful.
Brian Harring
> One other enhancement I thought might be good is to have a
> Model.pre_init signal
Such a signal exists already last I looked, same for post_init...
Please clarify.
> Other similar class-based
> receiver lists might speed things up in some cases; the down-side is
> that each receiver list has some overhead, even if empty.
Please see ticket 4561... my attack of dispatch was to disable it when
nothing was listening, dynamically re-enabling it when there was a
listener. Via that, there is *zero* overhead for disconnected signal
pathways- slight overhead addition when something is listening
(specifically addition of another function call), but pretty much
neglible.
Suspect you and I came at it from different pathways; my plan was to
target common optimization (model initialization, 25% boost when no
listener connected), then work my way down.
The next step I was intending, presuming that patch ever got commited,
was to move actual connect/send directly into the signal instances
including the arg checking; might I suggest merging functionality of
the two?
Literally, you're doing the second step I was after, while the first
step levels a pretty significant gain for scenarios where signals
aren't currently used.
~brian
Jeremy Dunck
> On Mon, Mar 24, 2008 at 10:55:03PM -0500, Jeremy Dunck wrote:
> > One other enhancement I thought might be good is to have a
> > Model.pre_init signal
>
> Such a signal exists already last I looked, same for post_init...
> Please clarify.
...That's rather embarassing. You're right; sorry for the muddle.
...
> Please see ticket 4561... my attack of dispatch was to disable it when
> nothing was listening, dynamically re-enabling it when there was a
> listener. Via that, there is *zero* overhead for disconnected signal
> pathways-
I see what you're doing there, and I do appreciate the zero-overhead
on no connections. I think the patch is a bit of a mind-bender, and
poking extra attributes onto existing functions is a bit scary. I
guess I'll ask a committer to weigh in on it?
Original response from Jacob:
"
Looking over what Brian's written and the dispatch code, it looks like three
small changes would let us simplify dispatching immensely:
1. Require the ``sender`` argument when connecting to a signal (i.e. don't allow
``Any`` sender any more).
2. Don't do any pattern matching on call signals; assume all listeners conform
to the api ``listener(signal, sender, **kwargs)``.
3. Simplify ``robustApply`` accordingly (i.e. don't consider
positional arguments).
With those changes, the dynamic wrapping Brian's talking about would be much
easier and none of the signal code I've written or seen would break (IIRC).
"
So, Jacob or other committer: is the wrapping approach to
pre/post_init/save appreciated? I'd be happy to clean up 4561 and
6814 into one patch if so.