Opened 15 years ago
Closed 15 years ago
#6965 closed (fixed)
urlize should be faster
| Reported by: | floguy | Owned by: | Andrew Badr |
|---|---|---|---|
| Component: | Template system | Version: | dev |
| Severity: | Keywords: | ||
| Cc: | floguy@…, andrew@… | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Running the urlize or urlizetrunc filter on some text that is very large, or running it many times seems to slow down the system more than it should.
Attachments (2)
Change History (14)
comment:1 Changed 15 years ago by
comment:2 Changed 15 years ago by
| Cc: | floguy@… added |
|---|---|
| Has patch: | set |
| Owner: | changed from nobody to floguy |
| Status: | new → assigned |
Changed 15 years ago by
| Attachment: | faster_urlize.diff added |
|---|
Improved speed to about 50x faster. Tests still pass.
comment:3 Changed 15 years ago by
From a quick skim of your code, it seems like you're changing how the escaping works - what if a URL has an ampersand in it? Potentially couldn't you break the HTML if it trimmed to a limit half way in an &?
comment:4 Changed 15 years ago by
| Patch needs improvement: | set |
|---|
Great point! I'm going to have to think about how to overcome that, since it'll probably foil this whole current scheme.
comment:5 Changed 15 years ago by
floguy, are you still working on this? Otherise, I can take a stab at it.
comment:6 Changed 15 years ago by
No, go ahead. I implemented it another way but it made the code almost as slow as the old version :-(
comment:7 Changed 15 years ago by
| Owner: | changed from floguy to devin |
|---|---|
| Status: | assigned → new |
comment:8 Changed 15 years ago by
| Owner: | devin deleted |
|---|
I gave my shot at this these last couple days, but the overhead seems to largely be in the regular expressions. punctuation_re especially seems clunky. I, however, don't know enough about regular expressions to optimize them.
comment:9 Changed 15 years ago by
| Owner: | set to Andrew Badr |
|---|---|
| Status: | new → assigned |
comment:10 Changed 15 years ago by
| Cc: | andrew@… added |
|---|---|
| Component: | Uncategorized → Template system |
| Patch needs improvement: | unset |
I created a patch that skips the regex match for most words. This resulted in 10x speed improvement on a test set of real posts. The reason for checking for '@' and ':' on line 98 is because I don't want to change the behavior for e.g. http://localhost/ even though it's not clear whether this should be supported or not (I don't think so). The len call was removed for good measure.
comment:11 Changed 15 years ago by
| Triage Stage: | Unreviewed → Ready for checkin |
|---|
Patch looks good. Existing tests will be enough so I it's ready to rock.
comment:12 Changed 15 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
I'd be +1 on no email-to-mailto by default and adding an extra filter
urlize_with_emailsthat supports email-to-mailto.