Code

Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#4561 closed (duplicate)

dispatch refactoring

Reported by: (removed) Owned by: nobody
Component: Core (Other) Version: master
Severity: Keywords: performance
Cc: jarek.zgoda@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Original ml discussion is here.

Short version; dispatch is a slow beast- basic design flaws follow

  • signals are sent regardless of whether or not something is listening
  • every emit/send requires searching for any matching sender, and matching signal receiver- additionally, requires looking up Any. The listeners change only when a new connection is added/goes out of scope, doing it every time at send is inefficient
  • signals, while singleton instances, aren't used for anything more then effectively a constant; meaning all of the logic is shoved into dispatcher; due to this, there is no easy place to define the set of args sent to listeners, it's completely dependant on each send invocation using an adhoc agreed to standard. Due to a signal being just a constant, there is no easy space to make signals faster via calculating up front the receivers to notify (it could be implemented in dispatcher.*, but that code is already ugly/messy, and feels a bit fragile).

Attached is a patch adding the basic intelligence to signals; this folds in ticket #4521 (basic signal class with str/repr). Roughly, adds the following logic:

  • for 'smart signals' (signals that are more then just constants), adds (enable|disable)_sender(sender) methods. In dispatcher.connect, if the signal is 'smart', signal.enable_sender(sender) is invoked- basically notifies the signal that something is actually listening, and what specifically is listening. In disconnect, a amtching disable_sender(sender) is added- same thing (notify that a receiver is going away).
  • Tracking of all known 'smart signals'; this is used by Any, to multiplex enable_sender calls down into each individual signal
  • a pre/post signal class, and func to generate the paired signals. This one is a bit more complex, and is the initial gain (aside from cleanup )from this refactoring. Basically, it modifies the sender on the fly, injecting pre/post dispatches if anything is listening- if nothing is listening, it leaves the target alone. Via this, I'm getting >25% faster Model instance instantiation for when nothing is listening for that specific model/signal.

pre_init, post_init, pre_save, post_save are updated to use the more efficient pre/post signal classes; the remaining django.db.models.signals.* signals are updated to use the new simple signal class (easier for debugging mainly). At this point, there should be no break in API either; basically is just shifting things around, and adding in a bit of delegation.

Patch is production usable at this point, although test coverage isn't yet 100%.

Thoughts/comments/nits welcome.

Attachments (2)

dispatch-smart-signals.patch (24.8 KB) - added by (removed) 7 years ago.
smart signals v1
dispatch-smart-signals-fix-bitrot.patch (24.8 KB) - added by (removed) 7 years ago.
dispatch-smart-signals, correct for bitrot.

Download all attachments as: .zip

Change History (6)

Changed 7 years ago by (removed)

smart signals v1

comment:1 Changed 7 years ago by Simon G. <dev@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Ready for checkin

Hmm.. looks good, I'll mark it as RTC as the comments on the list suggest that everyone's most interested in refactorisation along these lines, and this seems to do that. Committers can drop it back to Accepted/DDN if they want to.

Changed 7 years ago by (removed)

dispatch-smart-signals, correct for bitrot.

comment:2 Changed 6 years ago by zgoda

  • Cc jarek.zgoda@… added

comment:3 Changed 6 years ago by telenieko

  • Resolution set to duplicate
  • Status changed from new to closed

See #6814 for more, I'll mark this one as dup as the other one figures in the Roadmap to 1.0

comment:4 Changed 6 years ago by jacob

(In [8223]) Major refactoring of django.dispatch with an eye towards speed. The net result is that signals are up to 90% faster.

Though some attempts and backwards-compatibility were made, speed trumped compatibility. Thus, as usual, check BackwardsIncompatibleChanges for the complete list of backwards-incompatible changes.

Thanks to Jeremy Dunck and Keith Busell for the bulk of the work; some ideas from Brian Herring's previous work (refs #4561) were incorporated.

Documentation is, sigh, still forthcoming.

Fixes #6814 and #3951 (with the new dispatch_uid argument to connect).

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.