-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Cache] Added PhpFilesAdapter #18832
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Depending on the OPCache configuration, the cache can persist when the file is modified. |
… issues, removed AbstractFilesystemAdapter and created FilesCacheHelper instead.
Thanks for the advice. I've added calls to I had some problems with HHVM due to facebook/hhvm#4797 . On HHVM once php file is included during the request it won't be reloaded no matter what (in this particular request). To avoid this when on HHVM I'm using file modification time to save cache item version number that I increment on save. It's also kept in file contents. When this numbers are different (checked using |
I am not sure if this is a good idea actually. If I understand the code correctly, users of this adapter will have to be extra carefully to not cache arbitrary data as they will otherwise be executed when the cache is loaded which could make applications prone to code injection. |
Actually, no, all data can be safely used with this adapter. I just did a quick search and symfony already does similar thing for dumping translations:
Anyway, here is how this works in this adapter: the data gets exported using var_export and serialize. PHP docs state that var_export For instance, let's take this nasty string: $data = '"\'; echo \'a\';"' . PHP_EOL . "\0" . '\n\r'; The cache adapter will create this file: <?php return unserialize('s:21:""\'; echo \'a\';"
' . "\0" . '\\n\\r";'); As you can see, it can handle everything fine, including null bytes, new lines and stuff. Of course it works just as fine for arrays, objects etc., for example this: $data = new \DateTime(); Generates: <?php return unserialize('O:8:"DateTime":3:{s:4:"date";s:26:"2016-05-23 08:59:28.000000";s:13:"timezone_type";i:3;s:8:"timezone";s:3:"UTC";}'); |
This is interesting and would need to be benched against other approaches (#18823 esp.) |
|
||
use Symfony\Component\Cache\Exception\InvalidArgumentException; | ||
|
||
class FilesCacheHelper |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not an abstract like you did previously?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to move some logic away (I couldn't just delete files for hhvm), and I just thought it's going to be more readable this way. Plus you could add tests to it this way if it will grow larger.
I can move it back to abstract if you prefer, I don't mind.
Yep, append-only for hhvm and native PHP: given the way opcache works (see http://jpauli.github.io/2015/03/05/opcache.html#understanding-the-opcache-memory-consumption), doing rewrites creates wasted memory, which we shouldn't. If one want rewrites, one should use another adapter IMHO. |
OK, I can modify it. Should it sent an exception if data is already there? Should it also sent exception when lifetime is not infinite? I think clear method should still be supported, just to make clear-cache and such work, right? |
It should return false: PSR-6 doesn't allow exceptions for error reporting. |
I made the adapter append-only. I did a quick test to compare my adapter with OpCacheAdapter from other PR. For my test I disabled revalidating opcache entirely (with revalidation enabled PhpFilesAdapter gets slower). I saved some values, each of them consisting of an array with 100 strings, 50 characters length each. I did my tests for PHP 7.0.6 on windows (with SSD, but that shouldn't matter for opcache). Here's saving 5000 values, then read randomly 10000 times:
For next tests I was reloading it until the response time settled down: Here's results for 10 values read randomly 10000 times:
Here's results for 5000 values read randomly 10000 times:
OpCacheAdapter is slightly faster, but requires saving everything at once during initialization (no appends, regular save not supported). |
Interesting. Can you please add apcu to the tested adapters, try on php 5.5&5.6, and/or share the test script? |
Here are results for php 7 including apcu (I've changed saving test a bit to better show the difference in required memory):
I can try with different php versions later this week. Here are files I've tested with: As for settings, I've disabled opcache verification and increased memory for both apcu and opcache. |
Looks promising, I think we could provide this adapter as an alternative to the apcu adapter. |
Thanks for clearing the code out. |
it's intentional, because per your benchs, hhvm should use apc for about the same perf benefit
good catch, I'm going to fix that |
Hum in fact I'm not sure I understand what you mean? The user can still overwrite values safely, isn't it? |
If I read this right, deleting key returns true, but does not call opcache_invalidate, so depending on settings it might mean value will still be returned on fetch. Do I understand this correctly? |
I though the same, then I tested the real behavior, and deleted files are always taken into account ( |
This PR was merged into the 3.2-dev branch. Discussion ---------- [Cache] Added PhpFilesAdapter | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This is taking over #18832. With a warm cache I get these numbers consistently (PhpArrayAdapter being the implem in #18823 ): ``` Fetching randomly 5000 items 10000 times: Symfony\Component\Cache\Adapter\FilesystemAdapter: 0.1367, 2 megabytes Symfony\Component\Cache\Adapter\PhpArrayAdapter: 0.0071, 2 megabytes Symfony\Component\Cache\Adapter\PhpFilesAdapter: 0.0389, 2 megabytes Symfony\Component\Cache\Adapter\ApcuAdapter: 0.0361, 2 megabytes ``` This means that the PhpArrayAdapter should be used first, then ApcuAdapter preferred over PhpFilesAdapter, then FilesystemAdapter. This is what AbstractAdapter does here. Also note that to get the cache working, one should stay within the limits defined by the following ini settings: - memory_limit - apc.shm_size - opcache.memory_consumption - opcache.interned_strings_buffer - opcache.max_accelerated_files Commits ------- 8983e83 [Cache] Optimize & wire PhpFilesAdapter 14bcd79 [Cache] Added PhpFilesAdapter
This PR adds PhpFilesAdapter, that works almost in the same way as FilesystemAdapter, but saving creates a .php file that is included on fetch (allowing opcache to kick in).
I later realized there is a similar PR: #18823 , but this one is not read-only and simpler (allows to use the regular cache interface), so might still be useful anyway.
At my company we use a similar approach to cache annotations, validations, doctrine metadata and queries. It's useful, because that way we can warm up cache during build. It performs much better than regular filesystem cache.
Writing files might actually be slightly slower than for regular cache, so it's meant for data that rarely changes.