url_alias optimizing doesn't work due to rss.xml [#29326]

Started up a new development site/module yesterday, and naturally got devel.module installed pretty quickly. Noticed tons and tons of calls in the form of SELECT src FROM url_alias WHERE dst = 'bloodlust_ccg', which I thought was odd since I didn't have path.module enabled. Also saw SELECT COUNT(pid) FROM url_alias which seemed like a smart optimization trick to me: "if there are no path aliases defined, then don't bother checking for 'em". This matches up with the logic in the bootstrap.inc code:

  if ($count === NULL) {
    $count = db_result(db_query('SELECT COUNT(pid) FROM {url_alias}'));
  }

  if ($action == 'wipe') {
    $map = array();
  }
  elseif ($count > 0 && $path != '') {

But, this optimization never comes into play, because "rss.xml" is always defined as an alias to node/feed. Of related interest is the in-core/closed http://drupal.org/node/22531. I'm not sure what the solution is here: a user can certainly a) enable the path.module, b) delete the rss.xml path, c) disable the path.module to get the optimization, but that's a bit too complicated. Perhaps the rss.xml alias should ONLY be defined when the path.module is enabled (ie. load the module, check for rss.xml, if not there, add it, etc.) That way, the optimization would be working dandy on a default installation.

Comment	File	Size	Author
#7	_29326_crap.patch	794 bytes	Morbus Iff
#7
#3	_29326_rssxml.patch	3.67 KB	Morbus Iff
#3

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Morbus Iff

they/them

CreditAttribution: Morbus Iff commented 28 November 2005 at 20:05

Morbus: yeah, we could do it the other way around though, make rss.xml the default (i.e. the menu callback in node.module), and drop node/feed in Drupal 4.7, then add an update in update.inc to alias node/feed to rss.xml rather than the other way around. like that, new Drupal 4.7 users don't "pay" for rss.xml but for 4.6 users nothing really changes

Comment #2

Morbus Iff

they/them

CreditAttribution: Morbus Iff commented 28 November 2005 at 20:05

(Grr. The above was from Dries, in IRC.)

Comment #3

Morbus Iff

they/them

CreditAttribution: Morbus Iff commented 7 December 2005 at 15:51

Assigned:	Unassigned	» Morbus Iff
Status:	Active	» Needs review

File	Size
_29326_rssxml.patch	3.67 KB

Patch attached:

* Removed rss.xml alias from default databases; no default aliases now.
* Redirects all upgraded 'node/feed's to 'rss.xml' instead.
* Replaces node/feed with rss.xml in node.module.

This was per Dries' comments, and satisfies the optimization technique. Without this patch, and today's HEAD, we get the following on node/1, per devel:

Executed 28 queries in 79.57 milliseconds. 
1.91	1	SELECT COUNT(pid) FROM url_alias
0.92	1	SELECT src FROM url_alias WHERE dst = 'node/1'
0.27	1	SELECT dst FROM url_alias WHERE src = 'comment/reply/1'
0.32	1	SELECT dst FROM url_alias WHERE src = 'user/1'
0.44	1	SELECT dst FROM url_alias WHERE src = 'node/add'
0.22	1	SELECT dst FROM url_alias WHERE src = 'devel/cache/clear'
0.19	1	SELECT dst FROM url_alias WHERE src = 'devel/execute'
0.22	1	SELECT dst FROM url_alias WHERE src = 'devel/variable'
0.2	1	SELECT dst FROM url_alias WHERE src = 'admin'
0.37	1	SELECT dst FROM url_alias WHERE src = 'logout'
0.63	1	SELECT dst FROM url_alias WHERE src = 'admin/menu'
1.1	1	SELECT dst FROM url_alias WHERE src = 'node/1/edit'

With this patch and a NEW install (ie., this optimization will mostly affect new users, and not those who are upgrading), it comes down to just the COUNT(pid) and then nothing else. For those who are upgrading, nothing changes, really: the same number of queries would be run (only for node/feed -> rss.xml, not rss.xml > node/feed).

I don't think, however, that any of the other RSS feeds we provide (ie., any other /feed URL) should be changed into an rss.xml type of structure - it's very hard to do that without polluting the namespace ("rss.forum.xml", "rss.term.13+17.xml", etc? all ugly, IMO). My take is that "rss.xml" is *the most important* RSS feed in Drupal (if it wasn't, node/feed wouldn't have received an rss.xml alias in the first place), and that's the reason why it gets special privileges. All of the other RSS feeds Drupal provides are quite ... "narrow" and serve a specific function that is evident in their URL paths.

Comment #4

moshe weitzman CreditAttribution: moshe weitzman commented 7 December 2005 at 16:36

looks clean and very useful. thx morbus.

Comment #5

chx CreditAttribution: chx commented 7 December 2005 at 17:50

Status:

Needs review

» Reviewed & tested by the community

Morbus says that as legacy module is not enabled by default, moving the node/feed to there would lead to silent link rot. He is somewhat right :) so, I think this deserves a commit.

Comment #6

Dries CreditAttribution: Dries commented 7 December 2005 at 20:57

Status:

Reviewed & tested by the community

» Fixed

Committed to HEAD. Thanks.

There are a number of small optimizations we could add; like, we should never do more lookup queries than the number of unique URL aliases in the database. When the size of the static cache equals the number of unique URL aliases in the database, we can stop doing lookups. I think this could be implemented with a 2-line change. Takers?