Major Performance Bottleneck in Path Resolution.

This is not correct.

Up to 4.6 we did exactly what you propose, and then the bottleneck was building the associative array for sites that have several hundreds or thousands of path aliases.

So, starting with 4.7, we rewrote the code so that it does db queries instead of building the associative arrays.

The old way was so bad that it took several seconds for some sites. I even have some 4.6 sites with a backport of the 4.7 patch to resolve this bottleneck.

You either have a good solution to a problem, or your benchmarking methodology is not accurate.

The best way to convince the rest of us, is to share the exact benchmarks that you did, in detail.

Thing like:

-Version(s) of Drupal you used.
- Before and after scenarios.
- Data set size.
- Tools used (e.g. ab/ab2, devel module, custom tools?)

This sounds to me to be an ideal solution for small websites, since small drupal installations are typically on shared hosting where MySQL queries are at a premium.

I'd like to make everyone aware of another discussion ongoing now: http://drupal.org/node/106559

Additionally, my question is: why can't we combine the current method AND this new method? If we initially query the number of alias, then we could compare it to some magic number. If the number of aliases is larger than the magic number, then the current method is used. Otherwise, this new method is used.

Here's a patch for Drupal 5.1 to implement my suggestion above. I don't recommend that this be put into the core. I'm just posting it here for other small site drupallers to help them out.

After I installed this patch, the number of mysql queries went from 159 queries executed in approx 250 ms to 89 queries executed in approx 150 ms. A nice little improvement for a site with only 100 nodes.

FYI, I set it up so that a site with less than 1000 aliases will cache them all, while a site with more than 1000 aliases will use the standard method. I don't know if this is the optimum cache size, but I didn't want it to be set too high to waste resources.

First of all, make that 1000 a setting under Performance somewhere. No hard coded numbers please.

Second, less queries is always good, but it has to be accompanied by a total reduction in page load time. I have seen cases where just building the associative array in 4.6 ate up way more time than order of magnitudes of the database query time.

Install the devel module and see how much each case is. Restart MySQL between the tests so as to rule out the query cache.

Also, look at how Drupal 4.6 used to do it, which is almost what you want to do (the code was in bootstrap.inc at the time before path.inc existed). Specifically, the functions drupal_get_path_map() and drupal_get_path_alias().

Another thing to look at is this comment in a related issue that is meant to cache some aliases if there is only a few of them.

Related to http://drupal.org/node/63635

For what it's worth, if the performance benefit is real then I would definitely support this for Drupal 6. The problem is that the Drupal 4.6 method is optimized for a site with just a few aliases, in which case I suspect that's fastest. The current method is optimized for sites that either do not have path module enabled (so no entries at all) or have zillions of them. Sites that have 10000 nodes but only 20 key aliases (like, say, Drupal.org) kinda get screwed. If we can have the code self-throttle, even if it's an admin-defined threshold, that lets us have our cake and eat it too. +1

I checked the page execution times, which are unchanged. My test site only has 100 aliases, so this may change for higher numbers of aliases. I don't have the time right now to test this with other numbers of aliases, but if someone else would like to try, please be my guest.

I have a feeling that page execution times will never decrease with this patch. The point at which the page execution times start increasing is where we should switch to the current system.

I'm content with this hack patch as it stands. However, I will continue working on it if others would like to see it mature.

@Crell
The other patch with whitelist/black list should also help with the case of 20 key aliases. These will be cached and one db I/O will get them.

@kevinweber
Here is an idea.

Install the devel module and enable the node/taxonomy generation that comes with it. Also install and enable pathauto and make sure it will create aliases for nodes.

Then creat 1,000 nodes.

You now have 1,000 aliases (at least).

However, you don't have a page with lots of aliases. Installing sitemenu will create one page that has many aliases on it.

Go ahead and benchmark that before and after your patch and see what the results are.

The final benchmark:

* Drupal 5.1
* 1000 nodes

---Current Path.inc.php---
* Front page has 329 database queries
* Average execution time is 2200 ms.

* Sitemap module has 218 database queries. (Most are path alias lookups.)
* Average execution time is 1500 ms.

---With the above patch---
* Front page has 213 database queries. (35% fewer)
* Average execution time is 3500 ms. (60% longer)

* Sitemap module had 24 database queries. (90% fewer)
* Average execution time is 2200 ms. (45% longer)

Conclusions: 1000 nodes is too many nodes for this "optimization". While the number of database queries decreased, the execution time increased dramatically. However, with 100 nodes (see comment #8) the number of database queries were reduced 45% with no noticeable change in execution time.

That is exactly what we found on 4.6, and why the 4.7 way was ported to some sites to get over this.

To give you and idea, here is the number of aliases on a large site (hundreds of thousands of page views per day).

mysql> select count(*) from url_alias;
+----------+
| count(*) |
+----------+
|    47752 |
+----------+

There was way too much time spend building the static associative array for every request, and way too much memory wasted.

What you can do is see the other approaches listed in links above (caching N aliases instead of doing individual queries, so it is one query for N (magic number with some settings).

Closed.