A little while back I was discussing conf_path() with effulgentsia and pwolanin, and they noted that a sites.php definition might allow us to skip some of the searching for settings.php during bootstrap.

If you watched Rasmus' keynote speech at Drupalcon Copenhagen, you may remember the section where he asked for "some kind of deploy mechanism" to save hunting around for settings.php on every request. Post deploy scripts are hard to do with Drupal since we let people add multisites at any time they like, but sites.php has the potential to be such a thing.

Looked at a strace (on my localhost) with straight core, just using sites.default.php as you'd get with a normal installation via the UI or drush. Note I left APC, xdebug and xhprof all enabled doing the straces, I think the mmap here are APC on a cache miss, but this doesn't really affect the rest of the output significantly:

6555  access("/home/catch/www/7/sites/sites.php", F_OK) = -1 ENOENT (No such file or directory)
6555  access("/home/catch/www/7/sites/9090.d7.7/settings.php", F_OK) = -1 ENOENT (No such file or directory)
6555  access("/home/catch/www/7/sites/d7.7/settings.php", F_OK) = -1 ENOENT (No such file or directory)
6555  access("/home/catch/www/7/sites/7/settings.php", F_OK) = -1 ENOENT (No such file or directory)
6555  access("/home/catch/www/7/sites/default/settings.php", F_OK) = 0
6555  lstat("/home/catch/www/7/sites/default/settings.php", {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
6555  lstat("/home/catch/www/7/sites/default", {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0
6555  lstat("/home/catch/www/7/sites", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
6555  open("/home/catch/www/7/sites/default/settings.php", O_RDONLY) = 9
6555  fstat(9, {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
6555  fstat(9, {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
6555  fstat(9, {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
6555  fstat(9, {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
6555  mmap(NULL, 15272, PROT_READ, MAP_SHARED, 9, 0) = 0x7fb1aa209000
6555  stat("/home/catch/www/7/sites/default/settings.php", {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
6555  munmap(0x7fb1aa209000, 15272)     = 0
6555  close(9)                          = 0

After this, I created a sites/sites.php file with the following line:

$sites['9090.d7.7'] = 'default';

Stracing again the output looks like this:

6897  access("/home/catch/www/7/sites/sites.php", F_OK) = 0
6897  stat("/home/catch/www/7/sites/sites.php", {st_mode=S_IFREG|0644, st_size=1782, ...}) = 0
6897  lstat("/home/catch/www/7/sites/sites.php", {st_mode=S_IFREG|0644, st_size=1782, ...}) = 0
6897  lstat("/home/catch/www/7/sites", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
6897  open("/home/catch/www/7/sites/sites.php", O_RDONLY) = 9
6897  fstat(9, {st_mode=S_IFREG|0644, st_size=1782, ...}) = 0
6897  fstat(9, {st_mode=S_IFREG|0644, st_size=1782, ...}) = 0
6897  fstat(9, {st_mode=S_IFREG|0644, st_size=1782, ...}) = 0
6897  fstat(9, {st_mode=S_IFREG|0644, st_size=1782, ...}) = 0
6897  mmap(NULL, 1782, PROT_READ, MAP_SHARED, 9, 0) = 0x7fa5a9c79000
6897  munmap(0x7fa5a9c79000, 1782)      = 0
6897  close(9)                          = 0
6897  access("/home/catch/www/7/sites/default", F_OK) = 0
6897  access("/home/catch/www/7/sites/default/settings.php", F_OK) = 0
6897  access("/home/catch/www/7/sites/default/settings.php", F_OK) = 0
6897  lstat("/home/catch/www/7/sites/default/settings.php", {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
6897  lstat("/home/catch/www/7/sites/default", {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0
6897  open("/home/catch/www/7/sites/default/settings.php", O_RDONLY) = 9
6897  fstat(9, {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
6897  fstat(9, {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
6897  fstat(9, {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
6897  fstat(9, {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
6897  mmap(NULL, 15272, PROT_READ, MAP_SHARED, 9, 0) = 0x7fa5a9c76000
6897  stat("/home/catch/www/7/sites/default/settings.php", {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
6897  munmap(0x7fa5a9c76000, 15272)     = 0
6897  close(9)  

Not as good as I'd hoped.

It removes the following:

6555  access("/home/catch/www/7/sites/9090.d7.7/settings.php", F_OK) = -1 ENOENT (No such file or directory)
6555  access("/home/catch/www/7/sites/d7.7/settings.php", F_OK) = -1 ENOENT (No such file or directory)
6555  access("/home/catch/www/7/sites/7/settings.php", F_OK) = -1 ENOENT (No such file or directory)

But, sadly, it adds some back because we're actually including sites.php now:

6897  stat("/home/catch/www/7/sites/sites.php", {st_mode=S_IFREG|0644, st_size=1782, ...}) = 0
6897  lstat("/home/catch/www/7/sites/sites.php", {st_mode=S_IFREG|0644, st_size=1782, ...}) = 0
6897  lstat("/home/catch/www/7/sites", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
6897  open("/home/catch/www/7/sites/sites.php", O_RDONLY) = 9

And there's this section which is two steps forward two steps back:

555  access("/home/catch/www/7/sites/default/settings.php", F_OK) = 0
6555  lstat("/home/catch/www/7/sites/default/settings.php", {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
6555  lstat("/home/catch/www/7/sites/default", {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0
6555  lstat("/home/catch/www/7/sites", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
6555  open("/home/catch/www/7/sites/default/settings.php", O_RDONLY) = 9

vs.

897  access("/home/catch/www/7/sites/default", F_OK) = 0
6897  access("/home/catch/www/7/sites/default/settings.php", F_OK) = 0
6897  access("/home/catch/www/7/sites/default/settings.php", F_OK) = 0
6897  lstat("/home/catch/www/7/sites/default/settings.php", {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
6897  lstat("/home/catch/www/7/sites/default", {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0
6897  open("/home/catch/www/7/sites/default/settings.php", O_RDONLY) = 9

So, still a bit messy.

So we could assume that sites/default/settings.php will always exist, if there's an entry for it in sites.php, i.e. this patch:

diff --git includes/bootstrap.inc includes/bootstrap.inc
index 0739332..c995bdc 100644
--- includes/bootstrap.inc
+++ includes/bootstrap.inc
@@ -395,7 +395,7 @@ function conf_path($require_settings = TRUE, $reset = FALSE) {
   for ($i = count($uri) - 1; $i > 0; $i--) {
     for ($j = count($server); $j > 0; $j--) {
       $dir = implode('.', array_slice($server, -$j)) . implode('.', array_slice($uri, 0, $i));
-      if (isset($sites[$dir]) && file_exists(DRUPAL_ROOT . '/' . $confdir . '/' . $sites[$dir])) {
+      if (isset($sites[$dir])) {
         $dir = $sites[$dir];
       }
       if (file_exists(DRUPAL_ROOT . '/' . $confdir . '/' . $dir . '/settings.php') || (!$require_settings && file_exists(DRUPAL_ROOT . '/' . $confdir . '/' . $dir))) {

Now it looks like:

8027  access("/home/catch/www/7/sites/default/settings.php", F_OK) = 0
8027  access("/home/catch/www/7/sites/default/settings.php", F_OK) = 0
8027  lstat("/home/catch/www/7/sites/default/settings.php", {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
8027  lstat("/home/catch/www/7/sites/default", {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0
8027  open("/home/catch/www/7/sites/default/settings.php", O_RDONLY) = 9

So that saves us one access.

Then you have to wonder while we're doing an access twice, to remove the double access, we can do this:

diff --git includes/bootstrap.inc includes/bootstrap.inc
index 0739332..3dfcc75 100644
--- includes/bootstrap.inc
+++ includes/bootstrap.inc
@@ -395,8 +395,9 @@ function conf_path($require_settings = TRUE, $reset = FALSE) {
   for ($i = count($uri) - 1; $i > 0; $i--) {
     for ($j = count($server); $j > 0; $j--) {
       $dir = implode('.', array_slice($server, -$j)) . implode('.', array_slice($uri, 0, $i));
-      if (isset($sites[$dir]) && file_exists(DRUPAL_ROOT . '/' . $confdir . '/' . $sites[$dir])) {
-        $dir = $sites[$dir];
+      if (isset($sites[$dir])) {
+        $conf = "$confdir[$sites][$dir]";
+        return $conf;
       }
       if (file_exists(DRUPAL_ROOT . '/' . $confdir . '/' . $dir . '/settings.php') || (!$require_settings && file_exists(DRUPAL_ROOT . '/' . $confdir . '/' . $dir))) {
         $conf = "$confdir/$dir";

Now we're down to one access:

8361  access("/home/catch/www/7/sites/default/settings.php", F_OK) = 0
8361  lstat("/home/catch/www/7/sites/default/settings.php", {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
8361  lstat("/home/catch/www/7/sites/default", {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0
8361  open("/home/catch/www/7/sites/default/settings.php", O_RDONLY) = 9

Now let's look at drupal_setting_initialize():

  if (file_exists(DRUPAL_ROOT . '/' . conf_path() . '/settings.php')) {
    include_once DRUPAL_ROOT . '/' . conf_path() . '/settings.php';
  }

.

So currently, we have duplicate file_exists() calls if you just do a normal install. If you use sites.php, there are three file_exists() for the same file!

Let's kill that file_exists() - if it fails, we're going to redirect you to the installer, where you should have been anyway. PHP will throw warnings in this case, but it doesn't actually break anything. It looks like install.php might need to add some extra checks here though so it doesn't try to include sites/default/settings.php for no reason.

694  lstat("/home/catch/www/7/sites/default/settings.php", {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
8694  lstat("/home/catch/www/7/sites/default", {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0
8694  open("/home/catch/www/7/sites/default/settings.php", O_RDONLY) = 9

And try include instead of include_once

8545  stat("/home/catch/www/7/sites/default/settings.php", {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
8545  lstat("/home/catch/www/7/sites/default/settings.php", {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0
8545  lstat("/home/catch/www/7/sites/default", {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0
8545  open("/home/catch/www/7/sites/default/settings.php", O_RDONLY) = 9

This is actually worse, need to look at that...

In either case, after the first request, if you have the PHP realpath cache set up properly (may not be the case on many hosts), then the lstat calls are cached, then best case we're down to:

8545  access("/home/catch/www/7/sites/sites.php", F_OK) = 0
8545  stat("/home/catch/www/7/sites/sites.php", {st_mode=S_IFREG|0644, st_size=1782, ...}) = 0
8545  stat("/home/catch/www/7/sites/default/settings.php", {st_mode=S_IFREG|0555, st_size=15272, ...}) = 0

At this point, I think that's as much as we could possibly get away with for Drupal 7. Will open a separate issue for Drupal 8.

This definitely still needs work for the installer, but CNR to see what the bot thinks.

Files: 
CommentFileSizeAuthor
#9 conf_before.png164.78 KBcatch
#9 conf_after.png186.25 KBcatch
#9 conf_path.patch1.97 KBcatch
PASSED: [[SimpleTest]]: [MySQL] 32,789 pass(es). View
#5 conf_path.patch1.96 KBcatch
PASSED: [[SimpleTest]]: [MySQL] 31,554 pass(es). View
conf_path.patch1.4 KBcatch
PASSED: [[SimpleTest]]: [MySQL] 31,553 pass(es). View

Comments

catch’s picture

catch’s picture

FileSize
1.96 KB
PASSED: [[SimpleTest]]: [MySQL] 31,554 pass(es). View

This fixes the warnings in the installer, installing via the UI worked fine with manual testing.

pillarsdotnet’s picture

pillarsdotnet’s picture

catch’s picture

This shouldn't have any functional changes at all apart from in very strange edge cases (like putting a site in sites.php that doesn't actually exist).

catch’s picture

Version: 7.x-dev » 8.x-dev
Issue tags: +needs backport to D7
FileSize
1.97 KB
PASSED: [[SimpleTest]]: [MySQL] 32,789 pass(es). View
186.25 KB
164.78 KB

Here's some benchmarks and more data:

Made the following file, conf.php:

/**
 * Root directory of Drupal installation.
 */
define('DRUPAL_ROOT', getcwd());

include_once DRUPAL_ROOT . '/includes/bootstrap.inc';
drupal_bootstrap(DRUPAL_BOOTSTRAP_CONFIGURATION);
print 'Hello world';

Additionally sites/sites.php looks like this:

$sites['9090.d8.8'] = 'default';

My D8 install is at http://d8.8:9090

Ran ab -c2 -n10000 http:d8.8:9090 with and without the patch:

8.x:


Concurrency Level:      2
Time taken for tests:   5.295 seconds
Complete requests:      10000
Failed requests:        0
Write errors:           0
Total transferred:      2220000 bytes
HTML transferred:       110000 bytes
Requests per second:    1888.53 [#/sec] (mean)
Time per request:       1.059 [ms] (mean)
Time per request:       0.530 [ms] (mean, across all concurrent requests)
Transfer rate:          409.43 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       0
Processing:     1    1   0.3      1      12
Waiting:        0    1   0.3      1      12
Total:          1    1   0.3      1      12

With the patch:


Concurrency Level:      2
Time taken for tests:   4.875 seconds
Complete requests:      10000
Failed requests:        0
Write errors:           0
Total transferred:      2220000 bytes
HTML transferred:       110000 bytes
Requests per second:    2051.42 [#/sec] (mean)
Time per request:       0.975 [ms] (mean)
Time per request:       0.487 [ms] (mean, across all concurrent requests)
Transfer rate:          444.74 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       2
Processing:     1    1   0.3      1      13
Waiting:        0    1   0.2      1      13
Total:          1    1   0.3      1      13

The script is tiny, my laptop isn't a clean benchmarking environment, so there is quite a lot of room for margin of error here, however running benchmarks several times I see the patched version consistently 5-10% faster except for a couple of outliers. xhprof suggests about the same saving (some from file_exists(), some from the surrounding logic).

While filesystem calls are cached (via PHP's stat cache and the filesystem itself), PHP does not cache misses:
http://php.net/manual/en/function.clearstatcache.php

You should also note that PHP doesn't cache information about non-existent files. So, if you call file_exists() on a file that doesn't exist, it will return FALSE until you create the file. If you create the file, it will return TRUE even if you then delete the file. However unlink() clears the cache automatically.

I don't know how operating system file caches work internally, so I don't know if they cache misses or not.

Additionally, if you have a multisite install (or just a lot of Drupal sites on the same server), you will be checking the existence of dozens or hundreds of files that are never going to exist every request - whatever caching might be in place for this, is going to get a bit polluted and have a bad hit rate.

Also attaching updated patch that applies cleanly to D8, and xhprof screenshots before and after.

catch’s picture

Title: conf_path() and drupal_settings_initialize() are too paranoid » Optimize settings.php discovery

Friendlier title.

moshe weitzman’s picture

This would be an improvement for sites hosted on slow filesystems like EBS. Also good for large multisites. Drupal Gardens is an example of both, I think.

Does this need more docs in sites.php or README? If not, is RTBC IMO.

pillarsdotnet’s picture

#9: conf_path.patch queued for re-testing.

catch’s picture

#9: conf_path.patch queued for re-testing.

andypost’s picture

After migration to core folder, we have a regression - install won't work if core is symbolic link to some other location.

.htaccess

# Follow symbolic links in this directory.
Options +FollowSymLinks

getcwd() in install.php now reports actual place of core/install.php so installer always reports no subfolder

example:
drupal.git - d8 repo

..
d8       - doc-root that holds symlink to core in repo

lrwxrwxrwx  1 andypost andypost    37 2011-11-06 07:23 core -> /home/andypost/drupal/drupal.git/core

and subfolder sites

~/drupal/d8$ ls -la sites/
итого 24
drwxr-xr-x 5 andypost andypost 4096 2011-11-06 03:59 .
drwxr-xr-x 6 andypost andypost 4096 2011-11-06 07:23 ..
drwxr-xr-x 4 andypost andypost 4096 2011-04-30 21:37 all
drwxr-xr-x 3 andypost andypost 4096 2011-11-06 04:05 default
drwxr-xr-x 4 andypost andypost 4096 2011-06-11 20:24 drupal8
-rw-r--r-- 1 andypost andypost 1785 2011-04-30 21:37 example.sites.php

But core won't find config in sites/drupal8 and reports

File system	
The directory sites/default/files does not exist.

Settings file	The settings file does not exist.
The Drupal installer requires that you create a settings file as part of the installation process. Copy the ./sites/default/default.settings.php file to ./sites/default/settings.php.

A current fix is to $_SERVER['SCRIPT_NAME'] to $_SERVER['SCRIPT_FILENAME'] - path in #484554-7: Stop relying on Apache for determining the current path

Probably symlinks to core should be filed into other issue...

sun’s picture

Status: Needs review » Needs work

How much of this is still relevant, given #1757536: Move settings.php to /settings directory, fold sites.php into settings.php + the other sites.php improvements that went into D8 already?

pounard’s picture

I guess some bits are still revelant if the D8 improvements still attempt some kind of discovery relaying on file_exists() calls.

Anyway, if this is not revelant anymore to D8 but if this can be optimized for D7, it would be good to keep this issue for D7 only.

YesCT’s picture

Issue tags: -i/o +i-o

(the slash in the i/o tag breaks the autocomplete from adding new tags)

andypost’s picture

Is this still an issue?

Berdir’s picture

D8 only attempts discovery if a sites.php exists, otherwise it hardcodes sites/default/settings.php. It's still slow if you have multisites, but most sites don't I guess...

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.