#3439 closed (fixed)
django.dispatch.* is grossly slow
Reported by: | (removed) | Owned by: | Adrian Holovaty |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Keywords: | ||
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Attached is a refactoring chopping off a good chunk of dispatch's overhead; hard to test it standalone, so testing was done for simple iteration over records and isolating the diff in the profiling for the refactoring.
Short version, attached patch shows 5x boost in dispatch speed; for iteration (just that, no rendering) over 53k records, time drops from ~15.5s to ~11.6s just by cleaning up dispatch.*
The internal data structs dispatch is using are still fairly innefficient- scaling will be an issue the more receivers there are for dispatched signals. So... can clean that up further, but there isn't any documentation re: the expectations of dispatcher. For example- order of connection to a signal, must the order be maintained for notifications at a specific sender/signal level? If I connect (in this order) func f1, func f2, both to object("foo") signal Any, must they be notified in order f1,f2?
I realize pydispatcher is a seperate project- that said, they've already moved onto 2.0, and there are several 1.0.x's for pydispatcher released already- djangos' bundled copy is still at 1.0.0. Whats the intention? Intending on keeping it in sync, or is it just a fork point?
Either way, the attached refactoring is not covered by tests at the moment- mainly since there are no tests in django for dispatcher. Would like to get feedback on the expectations of dispatcher (f1/f2 from above for example) prior to writing those tests, since the implementation (and google for that matter) lack any material on it.
Finally, due to the massive number of calls to send (thus to getAllReceivers and friends), normal seperation (getAllReceivers using getReceivers for example) was intentionally broken down a bit- in other words, inlined a few functions. The codes invoked enough that the overheads seperation really isn't worth it, especially if you're not actually using signals for the most part (thus dispatcher is pretty much pure time waste).
Attachments (2)
Change History (8)
by , 18 years ago
Attachment: | dispatch-refactoring.patch added |
---|
comment:1 by , 18 years ago
by , 18 years ago
Attachment: | dispatch.patch added |
---|
import of upstream tests, fix deregistration bug
comment:2 by , 18 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 18 years ago
Minor optimization I missed, although it's quadratic worst case correction; in _killBackref
while senderkey in receivers_list: try: receivers_list.remove(senderkey) except: break
is... certifiably insane. Utterly.
No idea how I missed that one...
receivers_list = [x for x in receivers_list if senderkey != x]
avoids quadratic worst case for if receivers_list is all senderkey; python lists are arrays, popleft forces everything beyond to be moved one to the left (worth noting context, and resolve_variable have the same whackyness).
comment:4 by , 18 years ago
This patch works quite well for me -- major props for tests, too :)
However, for some reason I can't understand at all, it seems to be failing under sqlite. I'm going to check the patch in anyway since the performance gains are measurable, but Brian, if you're still following this thread, can you *please* take a look at it under sqlite and try to figure out what's going on?
comment:5 by , 18 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
(In [4588]) Fixed #3439: vastly improved the performance of django.dispatch (and added tests!). Thanks to Brian Harring for the patch. Note that one of the new tests fails under sqlite currently; it's not clear if this is a sqlite problem or a problem with the tests, but it appears not to be a problem with the dispatcher itself.
comment:6 by , 18 years ago
Well... I guess since you said please. ;)
Which test is failing? The code *should* be totally unaware of sqlite also- quick grep of backends namespace, nothing in there seems to know of dispatch, so I'm semi at a loss why you're running into issues. Additionally... my testing was strictly against sqlite, so I'm a fair bit curious ;)
dispatcher refactoring round 1.