May 2005 - Development - lists.drupal.org

[drupal-devel] Bayosphere
by Dries Buytaert 15 May '05

15 May '05

http://bayosphere.com/node/28 http://bayosphere.com/why-drupal Might be worth a main page item! -- Dries Buytaert :: http://www.buytaert.net/

4 3

[drupal-devel] Cleaner Directory structure
by Jeremy Epstein 15 May '05

15 May '05

On 5/12/05, K B <kbahey(a)gmail.com> wrote: > All this does not need to go in CVS as it is. The CVS can largely > remain as it is. What the developers see when committing code and what > the end user (site admin) sees are two different things. > > The script that creates the tarball can deal with all this and > everything else remains the same. Not sure that I understand you there. The current situation in Drupal is that the contents of the tarball are exactly the same as the contents of CVS (except that CVS is more up-to-date). Are you saying that this will no longer be the case? Do you mean that checking out Drupal core will now have a different result to downloading a tarball of Drupal core? My understanding was that the Drupal core CVS repository would have everything under /public_html, and that the Drupal core tarball would have the same (this is how it is now). Why would you want to change this? Doing so would only confuse people. Jaza.

6 8

[drupal-devel] Idea - move help text out of code into database
by puregin 15 May '05

15 May '05

I'm going to re-float here on both lists the idea of having each module's help texts provided within the Drupal database, as opposed to being hardcoded in. Advantages: the help text documentation would be - easier to maintain - easier to translate - easy to tailor to user's experience level and preferences (tag each helptext with a 'User experience level" vocabulary and select the text according to the user's preferences) I think it would make the code easier to maintain also. Code review and documentation review could occur independently. Comments? Djun

7 9

[drupal-devel] [bug] file_check_location is not working on Windows 2000
by Dries 15 May '05

15 May '05

Issue status update for http://drupal.org/node/10885 Project: Drupal Version: cvs Component: file system Category: bug reports Priority: critical Assigned to: Anonymous Reported by: ccourtne Updated by: Dries Status: patch Don't we use file_create_path for that (it appends the 'files'-bit)? Dries Previous comments: ------------------------------------------------------------------------ September 16, 2004 - 15:46 : ccourtne Currently file_check_location is always returning false when passed a file as the $source path. If a directory is passed in $soruce file_check_location works. I've tracked this down to realpath returning an empty string when passed a relative path which points to a file. This probably goes unnoticed if you are using the file.api's to construct relative paths to the druapl file directory since check_location failing just causes the durpal file directory to get added which is what you wanted in the first place. Replacing this line $source = realpath(dirname($source)); with this $source = realpath(dirname($source)); fixes it on my windows box using PHP 4.3.8. I don't have things set up to do patches on my windows boxes here at work yet. I'll submit a formal patch tonight. ------------------------------------------------------------------------ September 16, 2004 - 15:47 : ccourtne Dag nabit. Replace $source = realpath($source); with $source = realpath(dirname($source)); Craig ------------------------------------------------------------------------ September 16, 2004 - 15:56 : Anonymous Attachment: http://drupal.org/files/issues/file-inc-file_check_location.patch (639 bytes) The sooner there are patches, the better it is... I tested this approach on my server and it works. Steef ------------------------------------------------------------------------ September 17, 2004 - 03:34 : ccourtne Attachment: http://drupal.org/files/issues/checklocation.patch (666 bytes) Here is the patch. ------------------------------------------------------------------------ February 23, 2005 - 02:30 : dan90(a)drupal.org This also fixed my file upload problems with some nonstandard paths under Linux/apache, php 4.3.4 drupal 4.5.2. maybe we should remove mention of windows 2000? ------------------------------------------------------------------------ March 1, 2005 - 19:32 : killes(a)www.drop.org Still applies. ------------------------------------------------------------------------ April 18, 2005 - 13:10 : ec Is this patch still to be use with actual Drupal 4.6.0 ? Thanks. eric ------------------------------------------------------------------------ May 7, 2005 - 12:35 : Wes Cowley I hand applied the patch to my Linux based installation and it solved the problem I was having with filemanager/attachment. I assume it would fix the same problem I had with upload.module since the symptoms were identical. ------------------------------------------------------------------------ May 7, 2005 - 12:36 : Wes Cowley Meant to add, that was a 4.6.0 Linux installaion that this patch cleared up. ------------------------------------------------------------------------ May 14, 2005 - 09:59 : larryjhs Thanks, still, I'm a bit of a php newbie -- where do I put the patch -- into which bit of script? If it's not too obvious, tell me and I won't try! But if it's obvious, please give me the location L ------------------------------------------------------------------------ May 14, 2005 - 10:19 : ec This patch seem still to apply to 4.6 (I'm using it on my linux box too and it solve my problem with upload.module) it apply on "file.inc" that you will find in includes directory on top of your drupal root. Put the patch in the same directory as file.inc go in this directory too and apply the patch like this "patch -p0 < the_patch_to_apply" where patch is the command on *nix box. If you're not confortable you can either edit the file.inc by hand and make the change yourself, in the patch file, line beginning with "-" is the one to remove, line beginning with "+" is the one to add, other lines are the context so you're sure to find the right line. Hope this help. Regards. ------------------------------------------------------------------------ May 14, 2005 - 10:23 : ec BTW, IMHO this patch should be committed ASAP as it's solve the problem with upload.module that can't handle private directory outside drupal root. Tested on my sites ans work (for now ;-) like a charm. Regards.

1 0

[drupal-devel] prune tables
by Dries Buytaert 15 May '05

15 May '05

Would it make sense to change the "Discard watchdog/access log entries older than" settings to use the number of SQL table rows, rather than the number of days? The problem? Again, at drupal.org, I had these set to "3 days" which I felt was short enough for the tables not to grow unwieldy. It wasn't until this week that I found out -- through extensive profiling -- that 3 days was too long. The point? Unless you are a Drupal developer, you won't profile your site. This means, most users have no clue about how much data is kept track of, or why their site becomes slower and slower. The question? How can we make it so that this doesn't happen? To me, having an option "Number of records to keep" rather than "Discard records older than" makes sense, because (i) it gives me control over the maximum number of table rows (ii) without having to worry about traffic bursts or a growing number of visitors (the cost is more likely to be constant over time). It gives me a clue about the performance/resource ramifications of certain features. Having a similar setting for the sessions- and history-table would make sense too. Would this degrade or improve usability? Suggestions? -- Dries Buytaert :: http://www.buytaert.net/

9 10

[drupal-devel] [feature] Recent nodes/comments block (patch attached)
by hba 15 May '05

15 May '05

Issue status update for http://drupal.org/node/19387 Project: Drupal Version: 4.5.2 Component: tracker.module Category: feature requests Priority: normal Assigned to: Anonymous Reported by: hba Updated by: hba Status: patch Attachment: http://drupal.org/files/issues/tracker.module.recent.posts.block.patch (1.59 KB) This patch adds a recents posts/nodes/comments block to tracker.module. It is better than the 'recent comments' block in comment.module in my opinion: this block includes recent nodes/images/events/etc. as well, and it uses less space. The patch is for 4.5.2, but I guess it would work with few adjustments (if any) on 4.6.x. hba

2 2

[drupal-devel] [bug] Search query containing / gives 404
by wulff 15 May '05

15 May '05

Issue status update for http://drupal.org/node/22850 Project: Drupal Version: cvs Component: search.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: wulff Updated by: wulff Status: patch Attachment: http://drupal.org/files/issues/search.module.patch (740 bytes) Searching for a date, e.g. "14/05/2005", results in a 404 error. search.module redirects to http://drupal.org/search/node/14%2F05%2F2005 which isn't handled correctly by mod_rewrite. The attached patch replaces all occurrences of '%2F' in the query with '/'. A test query for '1/2 1~2' results in a redirect to http://drupalhead.local/search/node/1/2+1%7E2 which is handled correctly by mod_rewrite. (Solution found via comment on http://ccca.nctu.edu.tw/~hlb/tavi/index.php?page=URL+Rewriting) wulff

3 3

[drupal-devel] 40 percent Drupal speedup
by Karoly Negyesi 15 May '05

15 May '05

Hi! After I have burned myself last time I presented some statistics, now I write a disclaimer first: these data are totally meaningless, and my assumptions based on them are only subjective opinions, nothing more. OK, now on for serious stuff. I ran ab -n 100 http://localhost/drupal/ two times (my homepage is node, I have a few basic nodes). First, split mode off, second on (document size was the same for both cases, and I conclude that split mode works as intended): min mean[+/-sd] median max Total: 277 364 173.7 306 1923 Total: 180 216 57.8 195 450 From this, I say split mode worths some attention. Regards NK

3 4

[drupal-devel] [bug] system_theme_listing() passes scalar to drupal_attributes()
by darix 14 May '05

14 May '05

Issue status update for http://drupal.org/node/22883 Project: Drupal Version: cvs Component: system.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: darix Updated by: darix Status: patch Attachment: http://drupal.org/files/issues/system.module_6.patch (1.36 KB) system_theme_listing() passes scalar to drupal_attributes(). this patch fixes it and wraps the params into array() i really wonder if drupal_attributes() should check for the type of the param? how expensive is an is_array()? darix

1 0

[drupal-devel] [task] Performance issues with path aliases
by mathias 14 May '05

14 May '05

Issue status update for http://drupal.org/node/22035 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: mathias Reported by: mikeryan Updated by: mathias Status: patch Attachment: http://drupal.org/files/issues/alias-query-on-demand2.patch (1.03 KB) Database schema updates for the applied patch [1]. [1] http://drupal.org/cvs?commit=16021 mathias Previous comments: ------------------------------------------------------------------------ May 4, 2005 - 19:10 : mikeryan See Investigate use of conf_url_rewrite [2] for context... The current core support for translating incoming path aliases to the internal Drupal path (drupal_get_path_alias) and substituting aliases when generating links to internal paths (drupal_get_normal_path) does not scale well with many aliases. The ease with which pathauto enables site administrators to generate large numbers of aliases exposes this issue, but it is inherent in the core implementation, because drupal_get_path_map reads in the entire url_alias table at bootstrap time. I'd like to discuss ideas on how to improve the performance... Most obviously, why not simply query the url_alias table as needed instead of loading the whole table? In the incoming case, only a single simple SELECT is necessary, which will always be more efficient than reading the table. In the outgoing case, there might be some slight performance advantage to caching the table with a small number of aliases, but the disadvantage can become huge as the alias table grows. One note - I've noticed that the src column in url_alias is not indexed, I think adding an index should significantly help the performance in the outgoing case if we were to do individual SELECTs. Any other thoughts? [2] http://drupal.org/node/21938 ------------------------------------------------------------------------ May 4, 2005 - 20:02 : mikeryan Attachment: http://drupal.org/files/issues/bootstrap.inc_2.patch (1.06 KB) What the hell, I went ahead and gave it a shot... On my home system, where I'm testing out 4.6.0, page loads have been taking several seconds, which I attributed to the fact that it's an old computer and I'm multi-tasking like crazy. I implemented my own suggestion, and now pages load in about one second, it makes an incredible difference (FWIW, my url_alias table has over 4000 rows). Patches attached, go give it a try... ------------------------------------------------------------------------ May 4, 2005 - 20:02 : mikeryan Attachment: http://drupal.org/files/issues/common.inc_11.patch (723 bytes) One attachment per note, that's tedious... ------------------------------------------------------------------------ May 4, 2005 - 20:03 : mikeryan Attachment: http://drupal.org/files/issues/path.module_0.patch (1.34 KB) ------------------------------------------------------------------------ May 4, 2005 - 20:03 : mikeryan Attachment: http://drupal.org/files/issues/database.mysql_4.patch (553 bytes) ------------------------------------------------------------------------ May 4, 2005 - 20:04 : mikeryan Attachment: http://drupal.org/files/issues/database.pgsql_2.patch (502 bytes) ------------------------------------------------------------------------ May 4, 2005 - 20:07 : mikeryan Note: I just noticed that the database.*sql patches showed an extra diff (not mine) to the location field in locales_source. I did my diffs against a freshly-updated DRUPAL-4-6, and had made the edits against a release from a few days ago... Those locales_source changes should probably be removed from my patches before applying. ------------------------------------------------------------------------ May 7, 2005 - 17:13 : mikeryan I upgraded Fenway Views [3] to Drupal 4.6.0 today, incorporating these patches. Performance is very noticeably improved. [3] http://fenway-views.com/ ------------------------------------------------------------------------ May 8, 2005 - 07:31 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/pathalias.patch (4.98 KB) I've merged the patch into one. Much more convenient. I also changed it to cvs as only there new features will be added. Upgrade path needs to be added. Mike, can you run some more tests? We are especially interested in "hard" data ie numbers with possibly error bars. it would be interesting to know how this patch affects sites that have only a few path aliases vs those with a lot of them. Also, how pages with a lot of node links (tracker) are affected. It might be worthwhile to add static caching in drupal_get_normal_path if it gets calle dmore than once for a link on one page view. ------------------------------------------------------------------------ May 8, 2005 - 11:21 : mikeryan Thanks for merging the patches, I wasn't aware you could patch multiple files at once. If this is accepted, of course, adding the index on src should be incorporated into updates.inc. In terms of hard data, I've never profiled PHP code - what tool(s) do you use? I couldn't find anything referenced in the PHP manual... re: pages with lots of links - on Fenway Views the big test is the calendar [4], and the performance improvement seems even more dramatic here. I don't know why - the url_alias table is read once per page, so I would expect as you do that preloading the table would tend to look better when you have a hundred internal links on the page. Maybe we're both underestimating exactly how fast a simple MySQL query on an indexed key can be (I'm using MySQL 4.0.20, BTW)... [4] http://fenway-views.com/calendar ------------------------------------------------------------------------ May 8, 2005 - 11:26 : killes(a)www.drop.org As we speak, Mathias is doing some tests. The tool of choice is usually apache bench. If you have a lot of url aliases, the generated array will be huge. Part of the improvement could be due to the fact that you need less memory now. ------------------------------------------------------------------------ May 8, 2005 - 11:28 : mikeryan Great, thanks! ------------------------------------------------------------------------ May 8, 2005 - 11:34 : Dries On drupal.org it takes 800 ms or more to generate a page. Of these 800 ms, only 4 ms is spent building the path alias map (incl. the SQL query time which takes about 1 ms). That is, building the map takes 0.5% of the total time which is neglible. In our url_alias table are only 321 aliases though. How many aliases do you have? What is more, when we build drupal.org's main page, we query this map 215 times. I believe your patch would introduce 215 SQL queries ... I'm afraid that if we'd apply your patch, we'd pay a serious performance penalty (unless we have many more aliases). Can you provide some numbers/measurements? ------------------------------------------------------------------------ May 8, 2005 - 11:37 : Dries Note: my measurements did not take into account the time spent searching the map. On the main page, we search this array 215 times. ------------------------------------------------------------------------ May 8, 2005 - 12:08 : puregin I believe that it's felt that adding path aliases would improve the Drupal documentation. This may well add 400+ paths to the URL aliases table on Drupal.org ------------------------------------------------------------------------ May 8, 2005 - 12:27 : Dries I spent some more time investigating this. The relevant code in bootstrap.inc is this: function drupal_get_path_alias($path) { if (($map = drupal_get_path_map()) && ($newpath = array_search($path, $map))) { return $newpath; } ... Turns out that the time to build the map is neglible (< 5ms, see previous comments), however the total time spent on 'path aliasing' it about 70ms per page! 50ms of these 70ms are spent on $map = drupal_get_path_map(). The remaining 20ms is spent on $newpath = array_search($path, $map). The first call to drupal_get_path_map() takes 3 to 5 ms, each subsequent call takes about 0.3 ms. Searching the array costs 0.1 ms. However, if you have to do this 130+ times, this adds up to a whopping 70 ms. Remind that we have 321 aliases in our map. As drupal_get_path_alias() is typically called from code>url(), which in turn is typically called from l() I set out to investigate the time spent in l(). Looks like l() easily gets called 120 times/page, and that each call to l() costs about 0.9 ms. That is about 2 or 3 times the cost of the /average SQL query/. The total time spent in l() is 115 ms! ------------------------------------------------------------------------ May 8, 2005 - 16:25 : adrian This is a complete aside, and probably off-topic, but I would like to mention that I was actually looking at rewriting url() to be able to handle 'external urls' , because I want to do aliases using the site subdomain, like for instance : http://category.sitename.com/article/title_goes_here -> node/23 Rewriting url to allow this would also allow us to use l() for external links of any kind, getting rid of a bunch of inline html. ------------------------------------------------------------------------ May 8, 2005 - 16:46 : chx adrian, I have written a patch for l to support external links but Ber said that a module like weblink shall handle those. ------------------------------------------------------------------------ May 8, 2005 - 17:03 : mikeryan I've got over 4000 aliases on Fenway Views - another pathauto user reported over 6000. I don't have a profiling tool to give timing data, but adding a quick counter shows that for the Fenway Views calendar [5], drupal_get_path_alias() is called 404 times and drupal_get_normal_path is called 227 times. And trust me, this page loads MUCH faster with my patch than it does with the path map. I think those SQL queries are a lot cheaper than one might expect... [5] http://fenway-views.com/calendar ------------------------------------------------------------------------ May 8, 2005 - 22:24 : mathias Attachment: http://drupal.org/files/issues/pathalias-with-caching.patch (5.62 KB) Some benchmarks, with MySQL and Drupal caching disabled. 2 SETS OF TESTS ======================================= 1) 500 nodes and aliases 2) 20,000 nodes and aliases 3 TYPES OF TESTS PER SET ======================================= 1) Baseline unmodified Drupal 2) The following change inside drupal_get_path_map(): - if (is_null($map)) { + if ($map === NULL) { $map = array(); // Make $map non-null in case no aliases are defined. 3) Pull aliases only when needed rather than loading the entire alias table. SET 1 - 500 Nodes and 500 Aliases using: ab -c 10 -n 100 [homepage] ======================================= Baseline Time taken for tests: 13.00 seconds Requests per second: 3.85 [#/sec] (mean) Transfer rate: 56.35 [Kbytes/sec] received is_null() VS NULL Time taken for tests: 12.463 seconds Requests per second: 4.01 [#/sec] (mean) Transfer rate: 57.63 [Kbytes/sec] received Per Instance Alias Lookup (32 additional queries) Time taken for tests: 11.502 seconds Requests per second: 4.35 [#/sec] (mean) Transfer rate: 63.30 [Kbytes/sec] received SET 2 - 20,000 Nodes and 20,000 Aliases using: ab -c 5 -n 50 [homepage] ======================================= Baseline Time taken for tests: 204.583 seconds Requests per second: 0.24 [#/sec] (mean) Transfer rate: 3.34 [Kbytes/sec] received is_null() VS NULL Time taken for tests: 127.629 seconds Requests per second: 0.39 [#/sec] (mean) Transfer rate: 5.35 [Kbytes/sec] received Per Instance Alias Lookup (32 additional queries) Time taken for tests: 28.788 seconds Requests per second: 1.74 [#/sec] (mean) Transfer rate: 23.59 [Kbytes/sec] received ANALYSIS ======================================= Converting 'is_null()' to '=== NULL' is a no brainer and results in a nice performance gain. And while per instance alias lookups also give a huge boost for site with thousands of aliases, they're benefits are entirely dependent on the number of system URLs and menu items visible per request. As noted above I tested the standard homepage which added 32 additional queries. A request for something like the admin interface would presumably show less benefits. I would've tested this, but I didn't know how to invoke an authenticated page request with 'ab'. A positive of this approach is we no longer would be storing all the aliases in memory. What other concerns do we have with this last approach? Am I properly testing the strain on the database? ------------------------------------------------------------------------ May 8, 2005 - 23:08 : Dries Can you try making the following changes? 1. To common.inc: function drupal_rebuild_path_map() { - drupal_get_path_map('rebuild'); + drupal_get_path_map(TRUE); } 2. To bootstrap.inc: -function drupal_get_path_map($action = '') { +function drupal_get_path_map($rebuild = FALSE) { static $map = NULL; - if ($action == 'rebuild') { + if ($rebuild) { $map = NULL; } That is, replace the string comparison with a boolean comparison. Not sure it is going to make a significant difference but it might be another micro-improvement. I'll test your patch shortly. ------------------------------------------------------------------------ May 9, 2005 - 06:21 : mathias Dries. Those changes had no real performance gain. 20,000 Nodes and 20,000 Aliases using: ab -c 5 -n 50 [homepage] ======================================= Baseline Time taken for tests: 204.583 seconds Requests per second: 0.24 [#/sec] (mean) Transfer rate: 3.34 [Kbytes/sec] received String VS Boolean Time taken for tests: 204.190 seconds Requests per second: 0.24 [#/sec] (mean) Transfer rate: 3.34 [Kbytes/sec] received ------------------------------------------------------------------------ May 9, 2005 - 07:01 : Dries Before this patch can be committed, we need to do more testing. I'd like to know how this behaves on sites with few or no path aliases, and on sites like drupal.org, with a modest amount of path aliases. The reason I'm asking is because MySQL is often the bottleneck and not PHP/Apache. This is the case on drupal.org, for example. The proposed patch moves some of the processing costs from PHP to MySQL. On drupal.org, the amount of SQL queries/page would double. Needless to say, this is somewhat scary. ;) More numbers would be appreciated. ------------------------------------------------------------------------ May 9, 2005 - 20:56 : mikeryan I'm curious, why test with MySQL caching disabled? Since much of the issue is the expense of making (potentially many) more queries, this wouldn't seem to reflect the performance gain in practice. The caching (compared to making the query each time) seems like overkill to me - with MySQL caching enabled, I would think this complicates the code for little (if any) performance gain. Did you do any profiling using my original (cacheless) patch? Thanks. ------------------------------------------------------------------------ May 9, 2005 - 23:23 : mathias In an ideal world everyone would be running a query cache but I wanted to see how this would hold up under the worst-case scenario. I tested the original patch which didn't use static variables and the amount of queries doubled. For example, to load the front page, Drupal queried the url_alias table 22 times looking for an alias for 'node'. There's no need to send duplicate queries to the DB since it seems that's where most bottlenecks seem to lie. ------------------------------------------------------------------------ May 10, 2005 - 16:41 : mikeryan re: worst-case scenario - I understand that. re: # queries - I would not want to assume adding to the number of queries is necessarily a significant performance hit (with database caching on, of course). I understand the suspicion - I've been around long enough to remember when you did everything you could to avoid making one single SQL query more than you absolutely had to. But modern databases (including MySQL) are very well optimized for this kind of application (lots and lots of small queries). And, I have seen real-life cases where adding caching of data that's already being cached somewhere upstream actually degrades performance. As long as we're examining this particular area of performance under a microscope, let's make sure we squeeze every millisecond of savings we can out of it - I'd just like to see the numbers for a cached database/no Drupal caching combination for comparison. Thanks. ------------------------------------------------------------------------ May 10, 2005 - 23:12 : mathias Dries asked me to do some tests using the drupal.org database. Once again it appears that plucking aliases out of the database as needed performed better than grabbing the entire alias table at the beginning of the request lifecycle, even if the alias table is small. Here's the numbers. using: ab -c 5 -n 50 [homepage] Drupal.org Baseline ============= Time taken for tests: 18.081 seconds Requests per second: 2.77 [#/sec] (mean) Transfer rate: 52.69 [Kbytes/sec] received 1 page request was 83 queries in 60ms for a cycle execution time of 255ms Drupal.org Per Instance Query ================== Time taken for tests: 16.536 seconds Requests per second: 3.02 [#/sec] (mean) Transfer rate: 56.63 [Kbytes/sec] received 1 page request was 125 queries in 65ms for a cycle execution time of 230ms Notice that the database does work a little harder to retrieve the queries one by one. And it also doesn't make sense to run 40 extra queries on a site with only 50 aliases. I think at this point we have a couple of options to explore: 1. Continue to try and squeeze out any other optimizations we can in the original path aliasing code. Maybe look at output buffering or a mechnism that'll allow us to start working the database resultset as soon data is fed into the pipe rather than waiting for the request to finish. 2. Try to identify the point (e.g., the number aliases) at which it becomes more effecient to fetch aliases and change patterns on the fly. As with most optimizations, the end result will probably be a combination of both. ------------------------------------------------------------------------ May 10, 2005 - 23:59 : Dries Looks like in all cases, performance actually improves. I've run a couple tests myself using the drupal.org database and measured performance improvements up to 15%. Of course, this was done on a machine where the MySQL database wasn't hammered on by 50+ concurrent requests. It would be appreciated if you could extend your patch with a couple lines of documentation to document the behavior (so people understand why we are doing it this way), the code and the API. Maybe add an entry to CHANGELOG.txt? Do we need an update for updates.inc? As for future work, I think we're better off looking at l(). As said, one call to l() takes about 0.9 ms and we easily do 100+ such calls per page. Caching at the level of l() is likely to buy us more but I don't know whether it is feasible. Maybe we can think of other optimizations too. If anything, it might be a good idea to add a TODO/NOTE to the PHPdoc of l(). Or, if you feel like messing with l() first, by all means! Count me in -- it's fun. :-) ------------------------------------------------------------------------ May 11, 2005 - 20:29 : mathias Attachment: http://drupal.org/files/issues/alias-query-on-demand.patch (5.72 KB) I think this patch is now commit-worthy. I did some more profiling using xdebug 2.0 [6] and kcachegrind [7] and sure enough, drupal_get_path_map() was a resource hog on sites with many aliases. This should be quite an improvement. I also found a couple other small optimizations such as replacing array_key_exists() with a NULL check inside the arg() function (before [8] / after [9]) and calling isset() in order to avoid calls to error_handler. [6] http://xdebug.org [7] http://kcachegrind.sourceforge.net/cgi-bin/show.cgi/KcacheGrindIndex [8] http://www.asitis.org/tmp/before_arg.png [9] http://www.asitis.org/tmp/after_arg.png ------------------------------------------------------------------------ May 12, 2005 - 02:07 : Dries That last patch doens't seem to work. None of my URLs are being substituted by their path alias. Also, mind to rename the src action to source? ------------------------------------------------------------------------ May 12, 2005 - 13:00 : mathias Attachment: http://drupal.org/files/issues/alias-query-on-demand_0.patch (7.5 KB) Doh! The previous version only worked when the path module was enabled. Here's a new one that works independent of path module. It's also a little faster now. 20,000 node and aliases: without patch: 0.24 requests per second with patch: 1.87 requests per second (was 1.74) The gains on drupal_org and sites with under 400 or so aliases is small. Actually those sites will spend more time in the DB and less time churning through PHP code. Further optimizations could be made here by defaulting to the old method of grabbing all aliases and looking for further PHP optimizations. Should we look into this or is that overkill? ------------------------------------------------------------------------ May 12, 2005 - 15:24 : Dries Attachment: http://drupal.org/files/issues/alias.patch (7.97 KB) I upgraded the patch to HEAD (didn't apply) but it still doesn't work ... my URLs are not replaced by their aliases. I attached the updated version of your last patch. No need to optimize this more so let's evaluate this first. I'll test it on drupal.org, which only has a few aliases. Let's hope other people will test this too.

1 0