Make Drupal faster - put admin functions in conditional include

I'm not convinced this is a good idea, but I'll think about it some more.

In general, I like this idea because it means that we don't waste resources loading and parsing functions that will not be used.

I like that it's optional; existing modules don't have 'admin' => TRUE and so keep on working (and using more memory).

I like that there's a way to load all includes so you have the entire namespace available if you need it.

It's easy to do because we already have the menu system as a central checkpoint.

A gigantic +1 to this

I think this is a variant of the more radical full split mode that chx did before.

I like this idea, since it saves loading code that will never be executed, for the majority of the site visitors.

The proof though is a benchmark though that confirms that the savings in term of execution time and/or memory is non trivial.

I will try to do a benchmark and see the effect.

As promised, here is a benchmark.

First off. The node_functions.txt file that is at the top is way out of date. So I added the new functions, and guessed which ones are admin and which are user. I am attaching the file here in case someone wants to test/benchmark. chx: please check if the file makes sense, or do some functions need to be swapped around.

This is a default Drupal install, Clean URLs, using today's HEAD, with only 3 nodes, all promoted to front page. Apache 2.0.55, PHP 5.1.6, MySQL 5.0.24, AMD 64 3000+, 64-bit kernel, 1GB. (but all that is fluff, what matters is the comparative results).

The file sizes are:

Before split:
105706 node.module

After split:
61263 node_admin.inc
44424 node.module

Using the command $ ab2 -c5 -n1000 http://example.com/

Before the split:

Document Path:          /
Document Length:        4186 bytes

Concurrency Level:      5
Time taken for tests:   93.856207 seconds
Complete requests:      1000
Failed requests:        0
Write errors:           0
Total transferred:      4700000 bytes
HTML transferred:       4186000 bytes
Requests per second:    10.65 [#/sec] (mean)
Time per request:       469.281 [ms] (mean)
Time per request:       93.856 [ms] (mean, across all concurrent requests)
Transfer rate:          48.89 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0   15  60.8      0     585
Processing:    89  452 322.3    441    5124
Waiting:        0  439 330.1    439    5067
Total:         89  467 328.2    455    5400

Percentage of the requests served within a certain time (ms)
  50%    455
  66%    515
  75%    556
  80%    578
  90%    638
  95%    683
  98%    832
  99%   1119
 100%   5400 (longest request)

After the split:

Document Path:          /
Document Length:        4186 bytes

Concurrency Level:      5
Time taken for tests:   89.88277 seconds
Complete requests:      1000
Failed requests:        0
Write errors:           0
Total transferred:      4700000 bytes
HTML transferred:       4186000 bytes
Requests per second:    11.22 [#/sec] (mean)
Time per request:       445.441 [ms] (mean)
Time per request:       89.088 [ms] (mean, across all concurrent requests)
Transfer rate:          51.51 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    7  47.4      0     558
Processing:    84  435 230.6    433    4152
Waiting:        0  429 235.3    431    4068
Total:         84  443 234.3    437    4326

Percentage of the requests served within a certain time (ms)
  50%    437
  66%    508
  75%    542
  80%    557
  90%    614
  95%    671
  98%    804
  99%    975
 100%   4326 (longest request)

Looks like a 4-5% improvement by my calculations.

The above results were with eAccelerator NOT enabled. With it enabled, the difference is far less marked.

Without the split:

Requests per second:    33.30 [#/sec] (mean)
Time per request:       150.158 [ms] (mean)
Time per request:       30.032 [ms] (mean, across all concurrent requests)

With the split:

Requests per second:    33.59 [#/sec] (mean)
Time per request:       148.852 [ms] (mean)
Time per request:       29.770 [ms] (mean, across all concurrent requests)

As far as memory is concerned, here are the results:

With eAccelerator turned on, the difference in memory utilization is negligible (112 bytes) because the script is pre-parsed, pre-compiled, and cached.

With eAccelerator off there is a significant difference:

Before split: 9969936 bytes
After split: 9201000 bytes
Savings: 768936 bytes

I can see that this would save a lot of memory for environments that have no op-code cache, if applied across the board, and a performance gain of 5% is not bad either. All this becomes irrelevant if an op-code cache is used though.

Note: this split breaks devel, since it needs to load some functions that are now in node_admin.inc. If we decide to go to that direction, then special modules (e.g. devel) would need to load modulename_admin.inc explicitly or something like that. So, the memory measurement is with some custom code in index.php.

I really like this addition. Would be great if this idea was extended to other parts like the help sections or whatever else to load as minimal amounts of code to do the given task.

This thread http://lists.drupal.org/archives/development/2007-01/msg00574.html is a good read. We should definitely extend this to all of Drupal. Killes did something similar for notify.module (which I blindly reverted) so a generic solution would be awesome.

I am willing to do more benchmarking if chx or someone else creates more module_functions.txt (splitting admin and user functions) for the modules that are enabled by default in Drupal.

If this 5% performance gain scales linearly for other modules, and the memory savings is also more, then it is really a goal worth pursuing.

Mandatory modules:
block
filter
node (already done)
system
user
watchdog

Modules enabled by default:
color
comment
help
taxonomy

Then the next step would be extending that to popular contrib modules (image, views, cck, ...etc.)

Great idea! I'd even extend it a little:
Instead of 'admin' => TRUE in the menu callback, have 'include' => array('admin'). menu_executive_handler would then just load all modulename_arrayvalue.inc files in the module dir, e.g. modulename_admin.inc for the above example.

By doing this, conditional loading could easily be extended to other pages/functions that are not needed in the majority of the callbacks/hook executions of a module.

@Frando, that's a great idea. This could be a huge performance gain, kick ass!

Great idea.

I was just thinking almost the same thing yesterday. The MODULENAME_functions.txt need only has the INCLUDENAME part, and the MODULENAME_INCLUDENAME.inc file name can be derived from all that.

MODULENAME_menu
MODULENAME_links
MODULENAME_admin_settings admin
MODULENAME_something something

So, we end up with MODULENAME.module (those that have no second argument), MODULENAME_admin.inc and MODULENAME_something.inc

The only caveat is being careful for possible name clashes with current modules that have their own .inc files. Mainly applies to contrib, not a concern for core though.

We can even take this idea a bit further, and include a MODULENAME.split file (or whatever suffix) with every module, and then site admins can decide whether to split or not depending on whether they are running an op-code cache or not, and use drupal.sh to do the split from the command line.

We now are back full circle to having the original split mode prototyped by chx a while back, but it is not forced on anyone, and is configurable (split into 1 .inc + 1.module, a few .incs, or each function in its own file).

+1 to Frando in #14. That's especially nice because of the menu_alter ability in Drupal 6, which means a module author who really wanted to get crazy could menu_alter another module's page and add its own include if, say, it only wanted to form_alter a single form. It would get potentially tricky, though, because we could easily end up ballooning the number of includes. Disk hits are more expensive than code parsing unless you're dealing with a fair amount of code, so each case would need to be tested. Splitting off admin callbacks is the "low-hanging fruit", though, that most modules would probably want to employ. Still, this could be a huge boon to performance on non-opcode-cached sites, which includes basically all shared hosts.

The catch, though, is that you need to specify both a module and a component if you have anything fancier than a standard toggle. Otherwise the menu system doesn't know which module it should be including from. So the option would have to look something like:

'#include' => array(
  array('modulename', 'extensionname'), 
  array('modulename', 'extensionname'),
),

I don't know what complexity that would add to the menu system. chx, are we just talking out of our asses here? :-)

I really like this idea if it can be made to work.

This now has its very own new issue: http://drupal.org/node/140218 Please continue the discussion there.