Edgewall Software

Ticket #183 (new enhancement)

Opened 5 years ago

Last modified 4 years ago

py:match template matching is inefficient

Reported by: anonymous Owned by: cmlenz
Priority: major Milestone:
Component: Template processing Version: 0.4.4
Keywords: Cc:

Description

I noticed that most py:match's are fairly simple: <py:match path="tagname">

But when we got to match, we try to match every single event against a complex path, basically calling each path's _test() method over and over.

Instead I borrowed a play from web browsers: index common paths (i.e. like CSS selectors) and then only match against them. So I keep a dictionary mapping nodenames->py:match templates. Rather than calling _test() against every single py:match, I only call _test() against the paths that matter.

I'm attaching a patch to this bug, which is not quite the final patch.

Attachments

fast-path.patch Download (12.2 KB) - added by alecf@… 5 years ago.
Patch to make py:match faster, first cut
fast-path.2.patch Download (12.2 KB) - added by alecf@… 5 years ago.
Patch to make py:match faster, minus syntax error
fast-path.3.patch Download (12.2 KB) - added by alecf@… 5 years ago.
ok, really without syntax errors, I promise!

Change History

Changed 5 years ago by alecf@…

Patch to make py:match faster, first cut

Changed 5 years ago by alecf@…

Patch to make py:match faster, minus syntax error

Changed 5 years ago by alecf@…

ok, really without syntax errors, I promise!

Changed 5 years ago by cboos

Just a quicknote to mention I tried the patch and, for the specific use case of Trac, it doesn't provide any speed-up (rather a very small slow-down, but I'm not sure it's significant). I didn't look closely at the patch - maybe it doesn't even affect the once=true style of matches, so that would explain it.

Changed 5 years ago by aflett

Ok, I've checked in the latest version into the branch.. on our site (freebase.com) this patch provides a 3x-6x improvement (!) - pages went from taking ~2-3 seconds to render down to 200-500ms, depending on the page size. Still not ideal, but a huge gain.

I'd be very surprised if there was any slowdown for other pages as a result of this patch. Do you have some numbers?

Changed 5 years ago by aflett

as of r817, this shoudl be good to go. Love to get it merged into genshi trunk!

Changed 5 years ago by cboos

I've just found that there's a bug with r819, visible in Trac when rendering either the preferences or the admin panels: the layout part is missing, only the panels are shown.

Have a look at e.g.  trac:source:trunk/trac/admin/templates/admin.html, only the <body><div id="content">...</body> will be rendered when using r819 (works fine with r818).

Changed 5 years ago by aflett

cboos, are you using the fast-path branch? Sounds like we need another test case.

Changed 5 years ago by aflett

also if you are using this branch, be aware that the new py:match rules now apply - the py:match rules have fundamentally changed on the trunk, starting I think in r810 or r816.

these changes got merged into the branch in r819, so you might want to try the r819 on the trunk to see if you're seeing the same problem.

Changed 5 years ago by cboos

Well, I thought I was quite clear in the comment:4 above:

  • r819 (match-fastpaths branch) fails,
  • r818 (trunk) works.

Sorry for not going too deeply into the details with a sample test case, I just wanted to hint that the branch is probably not yet ready for merging ;-)

You can pretty easily see the problem by yourself by installing the latest Trac ( [trac 6792/trunk]) and going to the "Preferences" panels (any user) or the "Admin" tab (when logged in with a user having TRAC_ADMIN or TICKET_ADMIN permissions).

Changed 5 years ago by aflett

ok, thanks - sounds like genshi needs another test case, I'll see what I can do.

Changed 5 years ago by aflett

Ok, I got a trac instance from SVN trunk, genshi with my branch, and I don't see any difference between the trunk and the branch on either the admin or the preference pages. What is supposed to be missing? I get a full <head> with lots of <link> tags, and what appears to be a pretty full body too.

Find me on freenode, #trac or #python-genshi - I'd love to resolve this without breaking trac!

Changed 5 years ago by markus

I'm experiencing the same problems. It seems like all xi:include directives get ignored when using the match-fastpaths branch. For me, this happens with any Trac page (not only for /admin and /prefs).

I'm running Trac 0.11dev-r6792 with tracd, Python 2.4.4, Genshi speedups disabled.

Changed 5 years ago by cmlenz

  • milestone changed from 0.5 to 0.6

This will have to wait until after the 0.5 release.

BTW, I'm getting a number of test failures (27 to be exact) with the current version of the branch.

Changed 5 years ago by anonymous

oh man. I'll have a fix no later than April 8th...not sure why the new test failures, but I've been scrambling to get the branch back in working order.

Changed 5 years ago by markus

AFAICT, running Trac with match-fastpaths@828 works like a charm. The problems described above have gone and all tests pass: 

-----------------------
Ran 604 tests in 7.008s

OK

Changed 5 years ago by Matt Chaput <matt@…>

I downloaded the fastpath branch and tried it with an application/template that does a lot of py:matches. Everything worked with the 5.0 svn code, but with the fastpath code the template is mostly blank. It seems that doing select('node()') inside a py:match returns nothing in the fastpath branch.

Changed 5 years ago by Matt Chaput <matt@…>

Oops, spoke too soon, whatever it is, it seems to be doing it in the latest mainline as well. I'll open a new bug.

Changed 4 years ago by cmlenz

  • component changed from General to Template processing
  • milestone 0.6 deleted

If anyone's still interested in this, I'd love to see a performance comparison of this approach against current trunk, which also has some substantial optimizations in the XPath engine for simple paths.

Add/Change #183 (py:match template matching is inefficient)

Author


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


Change Properties
<Author field>
Action
as new
as The resolution will be set. Next status will be 'closed'
to The owner will change from cmlenz. Next status will be 'new'
The owner will change from cmlenz to anonymous. Next status will be 'assigned'
 
Note: See TracTickets for help on using tickets.