wordpress-opcache icon indicating copy to clipboard operation
wordpress-opcache copied to clipboard

Fixed file not found errors.

Open sun opened this issue 3 years ago • 2 comments

Problem

  • Analyzing the site performance with Query Monitor, a lot of PHP errors are appearing in the error log, because the object cache tries to read files that do not exist.

Proposed solution

  1. Test whether the file exists before including it. Avoid error suppression.

Notes

  • It can be argued that the file_exists() involves additional I/O because of an extra file stat. But PHP's error handling also does not come for free.

sun avatar Aug 24 '22 10:08 sun

Thanks for this, @sun I must admit I haven't looked at this for a long time. Your solution looks good, though a bit of Googling seemed to suggest that using is_file() instead of file_exists() should be better for performance (apparently even better than error suppressing with @ as is done now).

  • https://stackoverflow.com/questions/1708768/file-exists-is-too-slow-in-php-can-anyone-suggest-a-faster-alternative
  • https://github.com/composer/composer/issues/8892
  • https://www.drupal.org/project/ctools/issues/1234410

EDIT: I notice just now that you've contributed to that Drupal discussion. So, what is your conclusion about which method to use?

elcobvg avatar Aug 25 '22 04:08 elcobvg

Oh, that was a decade ago… 🙈

  • file_exists() also works for symlinks, but that shouldn't be relevant here.
  • file_exists() additionally checks whether it might be a folder or symlink, which might be slower than is_file().
  • Most of the linked issues conclude that the benchmarks were flawed. When properly invoking clearstatcache() upfront, the performance of file_exists() and is_file() does not differ.
  • We should ensure to use absolute paths to avoid PHP checking the path for each include_path.
  • In any case, the stat cache does not cache missing files. Repeated checks for missing files are slow. Probably less of an issue in our case, because every call to wp_cache_get() should be followed by a wp_cache_set().
    • However, in my testing I continued to see errors for missing files though - which ultimately triggered this issue. So it seems like this case happens more frequently. A potential workaround for that would be to maintain a separate cache item for cache misses.

sun avatar Aug 25 '22 10:08 sun