Goal

  • Leverage APC when it is available.

Use-cases

  • Symfony\Component\ClassLoader\ApcUniversalClassLoader

  • Drupal\Core\Cache\ApcBackend

    • Allow to use it for assorted cache bins.
  • APC user variables? We surely must have use-cases in core that could leverage those?

Related issues/projects

Files: 
CommentFileSizeAuthor
#56 1807662-56.patch12.93 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 58,202 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#49 1807662-49.patch12.58 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 57,997 pass(es), 10 fail(s), and 12 exception(s).
[ View ]
#47 1807662-47.patch12.57 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 55,491 pass(es), 10 fail(s), and 14 exception(s).
[ View ]
#47 interdiff-1807662-47.txt677 bytesdamiankloip
#45 1807662-44.patch12.35 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 55,458 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#45 interdiff-1807662-44.txt1.3 KBdamiankloip
#43 1807662-43.patch12.2 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 55,468 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#43 interdiff-1807662-43.txt9.82 KBdamiankloip
#34 platform.apc_.34.patch17.43 KBsun
FAILED: [[SimpleTest]]: [MySQL] 42,928 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#34 interdiff.txt6.67 KBsun
#31 platform.apc_.31.patch15.55 KBsun
FAILED: [[SimpleTest]]: [MySQL] 42,125 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#31 interdiff.txt5.38 KBsun
#26 platform.apc_.26.patch14.8 KBsun
FAILED: [[SimpleTest]]: [MySQL] 42,112 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#26 interdiff.txt3.79 KBsun
#25 platform.apc_.24.patch14.8 KBsun
FAILED: [[SimpleTest]]: [MySQL] 42,116 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#25 interdiff.txt8.66 KBsun
#16 platform.apc_.16.patch14.51 KBsun
FAILED: [[SimpleTest]]: [MySQL] 42,126 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#16 interdiff.txt3.73 KBsun
#13 platform.apc_.13.patch11.85 KBsun
FAILED: [[SimpleTest]]: [MySQL] 42,077 pass(es), 5 fail(s), and 4 exception(s).
[ View ]
#13 interdiff.txt3.5 KBsun
#12 platform.apc_.12.patch14.22 KBsun
FAILED: [[SimpleTest]]: [MySQL] 41,873 pass(es), 14 fail(s), and 0 exception(s).
[ View ]
#12 interdiff.txt3.24 KBsun
#10 platform.apc_.10.patch14.22 KBsun
FAILED: [[SimpleTest]]: [MySQL] 41,874 pass(es), 14 fail(s), and 0 exception(s).
[ View ]
#7 platform.apc_.3.patch28.44 KBsun
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch platform.apc_.3.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

Not sure I care if we have cache backends for it, but I do like the idea of using apc when it's available. For example, this patch is quite helpful when your files directory is on a distributed file system:

https://github.com/pressflow/7/pull/16/files

I would argue that we should automatically use ApcUniversalClassloader if APC is available. Don't make me edit a configuration variable when APC is clearly the better option. I would also argue that we should automatically use ApcBackend when APC is available and that both of those options should be able to be overridden in settings.php.

Except that APC class loader would need a cache flush every time you touch a file or class name in dev. Ideally if we got a dev/prod toggle enabled we could auto-select the APC class loader if available in prod mode only. Related: #1537198: Add a Production/Development Toggle To Core

@cweagans - Using APC as a cache backend by default is insane. No way we're doing that.

Crell is right about the autoloader. It's something you have to be aware of, so it shouldnt be the default.

Why APC? xcache2 more powerful
How it could work on shared hosts with different config options?

agree with #5
we should not integrate one op-code cacher feature - as framework has to be compatible and able to connect to any of them via common api layer

Title:[meta] Built-in APC support in coreBuilt-in APC support in core
Assigned:Unassigned» sun
Status:Active» Needs review
StatusFileSize
new28.44 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch platform.apc_.3.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Added Cache\ApcBackend from APC module.
Cleaned up apc stub functions.
Removed APC statistics support.
Fixed phpDoc, relocated methods.
Added visibility to methods.
Fixed missing method visibility in Cache classes.
Other Cache classes clean-ups.
Basic upgrade of getMultiple() and set() methods.
Added ApcBackendUnitTest.
Fixed bogus @file declaration.
Fixed bogus phpDoc in DatabaseBackend.
Upgraded ApcBackend for cache tags.
ApcBackendUnitTest passes, but UnitTestBase is broken; added lots of todos and debugging code. :(

We should probably back out the unrelated fixes into a separate issue.

Status:Needs review» Needs work

The last submitted patch, platform.apc_.3.patch, failed testing.

StatusFileSize
new14.22 KB
FAILED: [[SimpleTest]]: [MySQL] 41,874 pass(es), 14 fail(s), and 0 exception(s).
[ View ]

Remove unrelated changes, and Merged in HEAD.

Status:Needs review» Needs work

The last submitted patch, platform.apc_.10.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.24 KB
new14.22 KB
FAILED: [[SimpleTest]]: [MySQL] 41,873 pass(es), 14 fail(s), and 0 exception(s).
[ View ]

Removed Simpletest debugging code. To be fixed over in #1436684: Remove static cache in drupal_valid_test_ua()

StatusFileSize
new3.5 KB
new11.85 KB
FAILED: [[SimpleTest]]: [MySQL] 42,077 pass(es), 5 fail(s), and 4 exception(s).
[ View ]

Sorry, that was the wrong patch. :-/

Status:Needs review» Needs work

The last submitted patch, platform.apc_.13.patch, failed testing.

We would need to be cautious about how this is done in order to not be detrimental in shared hosting situations where the default apc.shm_size (if enabled at all) may not be sufficient. Excessive cache empties and reloads don't help anyone.

I would argue that we don't want full caching in APC by default; perhaps limit it to Symfony\Component\ClassLoader\ApcUniversalClassLoader by default so as to not eat up too much of the segment size while still providing a good performance boost.

Then make the cache backend for cache bins optional, for the user to configure as they see fit (and have apc segment memory available.

On a more philosophical note - why should the cache backend be in core? Anyone savvy enough to configure APC (admittedly simple) and configure the bins in settings.php is savvy enough to download it from contrib. What do we gain by providing the backend in core? And why just APC and not Xcode2, memcache, eaccellerator etc. etc.?

Status:Needs work» Needs review
StatusFileSize
new3.73 KB
new14.51 KB
FAILED: [[SimpleTest]]: [MySQL] 42,126 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

APC is maintained by the PHP core team. It is very actively maintained. It is available for all platforms and often comes pre-bundled/pre-installed even. Most of Drupal's competitors have built-in APC support since ages. Enough said.

Fixed unit tests are not able to detect PHP CLI, since run-tests.sh masquerades as Apache.
Fixed run-tests.sh bogusly detects unmet requirements as fatal error.

Status:Needs review» Needs work

This probably needs a comment in default.settings.php describing how to configure different cache bins to use APC with some examples (like the database settings). If the documentation isn't included in default.settings.php, then at least a URL for the documentation page on drupal.org should be added.

+++ b/core/lib/Drupal/Core/Cache/ApcBackend.php
@@ -0,0 +1,379 @@
+    // First we determine the prefix from a setting.
+    $prefix = self::getPrefixSettingForBin($this->bin);

Just to be on the safe side, this should probably be static::, not self::, so that we get late static binding. There's very few reasons to still use self:: in PHP 5.3.

+++ b/core/lib/Drupal/Core/Cache/ApcBackend.php
@@ -0,0 +1,379 @@
+    // When we are in testing mode we add the test prefix.
+    // @todo drupal_valid_test_ua() should *always* contain the correct prefix,
+    //   but does not due to Simpletest bugs.
+    // @see http://drupal.org/node/1436684
+    if ($test_prefix = drupal_valid_test_ua()) {
+      $prefix = $test_prefix . '::' . $prefix;
+    }
+    elseif (isset($GLOBALS['drupal_test_info'])) {
+      $prefix = $GLOBALS['drupal_test_info']['test_run_id'] . '::' . $prefix;
+    }

Can't we externalize this logic? Two global calls here make me cringe.

+++ b/core/lib/Drupal/Core/Cache/ApcBackend.php
@@ -0,0 +1,379 @@
+  protected static function getPrefixSettingForBin($bin) {
+    $prefixes = variable_get('cache_prefix', '');
+
+    if (is_string($prefixes)) {
+      // Variable can be a string, which then considered as a default behavior.
+      return $prefixes;
+    }
+
+    if (isset($prefixes[$bin])) {
+      if (FALSE !== $prefixes[$bin]) {
+        // If entry is set and not FALSE, an explicit prefix is set for the bin.
+        return $prefixes[$bin];
+      }
+      else {
+        // If we have an explicit false, it means no prefix whatever is the
+        // default configuration.
+        return '';
+      }
+    }
+    else {
+      // Key is not set, we can safely rely on default behavior.
+      if (isset($prefixes['default']) && FALSE !== $prefixes['default']) {
+        return $prefixes['default'];
+      }
+      else {
+        // When default is not set or an explicit FALSE, this means no prefix.
+        return '';
+      }
+    }
+  }

Again, can't the prefix be externalized? It's basically the same thing as passing in a DB connection to the database-backed cache driver.

+++ b/core/lib/Drupal/Core/Cache/ApcBackend.php
@@ -0,0 +1,379 @@
+  protected function binKey() {
+    return $this->prefix . $this->bin . '::';
+  }

I hereby decree this method shall be pronounced "Binky". :-)

+++ b/core/lib/Drupal/Core/Cache/ApcBackend.php
@@ -0,0 +1,379 @@
+  public function deleteMultiple(array $cids) {
+    apc_delete(array_map(array($this, 'key'), $cids));
+  }
+
+  /**
+   * Implements Drupal\Core\Cache\CacheBackendInterface::flush().
+   */
+  public function flush() {
+    $iterator = $this->getAll();
+    foreach ($iterator as $key => $data) {
+      apc_delete($key);
+    }
+  }

I'm curious why you're doing the fancy functional approach for deleteMultiple() but not for flush()? It's not a blocker I guess, I'm just curious why you wouldn't do both the same way.

+++ b/core/lib/Drupal/Core/Cache/ApcBackend.php
@@ -0,0 +1,379 @@
+// PHP CLI uses a different php.ini in which APC may not be enabled.
+// Create stub functions to prevent fatal errors.
+if (drupal_is_cli() && (!extension_loaded('apc') || !ini_get('apc.enable_cli'))) {
+  function apc_fetch($key, &$success) {
+    $success = TRUE;
+    return FALSE;
+  }
+  function apc_store($key, $var) {
+    return FALSE;
+  }
+  function apc_delete($key) {
+    return FALSE;
+  }
+  function apc_inc($key, $step, &$success) {
+    $success = TRUE;
+    return FALSE;
+  }
+}

Why is this code outside of the class? That's a no-no.

Most of Drupal's competitors have built-in APC support since ages. Enough said.

Most of Drupal's competitors are inferior to Drupal. That's hardly a justification for extra core bloat.

Don't get me wrong - I like APC a lot. I use it on all my projects. I don't, however, see any good justification for adding more core bloat... Given that the cache backend is intended to be pluggable, I ask again - why are we putting this backend in core, and not contrib? Alternatively, what justification allows this one in core, but not other stable caching backends?

Don't bloat the core up people!

I don't, however, see any good justification for adding more core bloat.

First, you need to define bloat. Disk space is cheap and I could care less about the few extra kilobytes that this would add. This code won't get loaded if it's not used, so if you don't want it, don't turn it on.

Second, having this available in core (especially if/when the config option is added to default.settings.php) ensures that people actually know about it. One of the biggest complaints that I hear about Drupal is that it's slow. I ask them "Well, have you turned on all the caching options built into core? Memcache? Boost? APC?" They usually answer, "Well, no...I haven't even seen that stuff."

Third, APC is the most common caching mechanism available, so this makes more sense for core than, say, the memcache backend.

Personally, I think it would be great if Drupal could cache things somewhere other than the database without having to download another contrib module. A module being enabled seems like more resource usage on every page request than simply setting a flag in settings.php

First, you need to define bloat. Disk space is cheap and I could care less about the few extra kilobytes that this would add. This code won't get loaded if it's not used, so if you don't want it, don't turn it on.

Probably a good idea. I would consider this bloat because it's code that is in core, and bound by core policies, that doesn't need to be. It places a burden on the core devs and those who help maintain core...

Second, having this available in core (especially if/when the config option is added to default.settings.php) ensures that people actually know about it. One of the biggest complaints that I hear about Drupal is that it's slow. I ask them "Well, have you turned on all the caching options built into core? Memcache? Boost? APC?" They usually answer, "Well, no...I haven't even seen that stuff."

There is definitely a problem here, but I see the solution in better documentation and training, personally.

Anyways, I will admit, we are off into the weeds from my initial main concern - I suggest we don't enable the APC cache bins by default, apart from perhaps class loading. Lots of shared hosting has 32M (default) or smaller apc.shm_size, and it's not hard to overrun that especially if the user runs several sites on the same account. The problem I see is that when APC is full, it flushes itself *completely*.

So if you end up filling the cache every few requests, the users could end up with a cold cache as often as not. It might actually be detrimental to performance in these situations.

@Crell:
Yeah, most of that prefix/bin/key code originates from the APC module and I didn't really touch it yet, since the logic going on there is quite advanced and debugging showed that some parts of it are indeed necessary.

The static stuff is definitely not required though - I'll convert that into a simple class property that's set up only once in __construct().

Regarding "externalizing" (I guess you mean injecting?) the prefix, I guess that's what #1764474: Make Cache interface and backends use the DIC is about?

The binKey() method should go away entirely IMHO - it just prefixes the $cid with a string - I don't see why we'd need a function call for that.

The key() method should be renamed to addPrefix(), I think.

The difference between deleteMultiple($cids) and flush() is that the former gets a known list of $cids passed, so those keys can be simply prefixed. flush(), however, needs to find all cache items in the store that belong to the prefix.

Lastly, the stub functions are a very interesting detail in this. (Apparently, a stub for ApcIterator is missing.) As the comment clarifies, APC might be turned off for PHP CLI but may be set as cache class via $conf['cache_classes'] in settings.php, in which case stuff like Drush and other CLI scripts would attempt to use it and break with fatal errors. Do you have any ideas for how we could resolve that in a better way?

Anyways, I will admit, we are off into the weeds from my initial main concern - I suggest we don't enable the APC cache bins by default, apart from perhaps class loading. Lots of shared hosting has 32M (default) or smaller apc.shm_size, and it's not hard to overrun that especially if the user runs several sites on the same account. The problem I see is that when APC is full, it flushes itself *completely*.

Read the patch. This is not enabled by default. You have to put stuff in settings.php to turn it on. Also? Shared hosting typically does not have APC enabled.

Status:Needs work» Needs review
StatusFileSize
new8.66 KB
new14.8 KB
FAILED: [[SimpleTest]]: [MySQL] 42,116 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Cleaned up APC prefix determination, variables, and methods.

StatusFileSize
new3.79 KB
new14.8 KB
FAILED: [[SimpleTest]]: [MySQL] 42,112 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Renamed all 'Apc' to 'APC', since it's an acronym and PHP itself uses 'APC', so 'Apc' would be inconsistent.

APC is a trademark :)

Status:Needs review» Needs work

The last submitted patch, platform.apc_.26.patch, failed testing.

+++ b/core/lib/Drupal/Core/Cache/APCBackend.php
@@ -0,0 +1,384 @@
+    $prefix = self::getPrefix($this->bin);

static:: please. :-)

+++ b/core/lib/Drupal/Core/Cache/APCBackend.php
@@ -0,0 +1,384 @@
+// PHP CLI uses a different php.ini in which APC may not be enabled.
+// Create stub functions to prevent fatal errors.
+if (drupal_is_cli() && (!extension_loaded('apc') || !ini_get('apc.enable_cli'))) {
+  function apc_fetch($key, &$success) {
+    $success = TRUE;
+    return FALSE;
+  }
+  function apc_store($key, $var) {
+    return FALSE;
+  }
+  function apc_delete($key) {
+    return FALSE;
+  }
+  function apc_inc($key, $step, &$success) {
+    $success = TRUE;
+    return FALSE;
+  }
+}

Hm. OK, I see what is going on here, now. I'm not a fan of the approach, but I don't have a better idea yet. At minimum, this sort of hackery should go in common.inc or something rather than in a class file.

And yes, by "externalize" I mean inject, vis, move that logic external to this class. If we can't do that yet due to another issue, let's drop a @todo on it with an issue link so we know to fix it later.

Status:Needs work» Needs review
StatusFileSize
new5.38 KB
new15.55 KB
FAILED: [[SimpleTest]]: [MySQL] 42,125 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

I'm not a fan of the approach, but I don't have a better idea yet. At minimum, this sort of hackery should go in common.inc or something rather than in a class file.

Me too. This code also originates from APC module. (It's really helpful to move something into core that was battle-tested already.)

The downside of moving this code elsewhere, however, is that we'd non-trivial code for checking whether the stubs are/will be needed. I'm perfectly aware that this completely violates PSR-*, but keeping the stubs in there seems to be the most simple and most elegant way to solve the problem, because the code is only loaded when actually needed and doesn't introduce complexity nor code debt elsewhere.

...but apparently, an APCIterator stub is still missing, and trying to add that in the same way yields:

Fatal error: Cannot declare class Drupal\Core\Cache\APCIterator because the name is already in use in Drupal\Core\Cache\APCBackend

So here's a different proposal that hopefully works better:

1) Check availability in the constructor, set a Boolean property, and implant a condition into the public cache API methods. If FALSE, do nothing.

2) Optional, later: If FALSE, turn the APCBackend silently into a MemoryBackend (i.e., built-in), so as to prevent data from being rebuilt multiple times on CLI.

Replaced self:: with static::.
Replaced stub functions with APCBackend::$available property.

Status:Needs review» Needs work

The last submitted patch, platform.apc_.31.patch, failed testing.

Ah, I like that a lot better. Agreed that a memory backend fallback would be even better. Could we do that by just making this class subclass off of the memory backend, and then if (!static::$available) { return parent::whatever(); } ?

Status:Needs work» Needs review
StatusFileSize
new6.67 KB
new17.43 KB
FAILED: [[SimpleTest]]: [MySQL] 42,928 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Added fallback to MemoryBackend if APC is not available.
Added test for APCBackend fallback to MemoryBackend. (YAY!)

Status:Needs review» Needs work

The last submitted patch, platform.apc_.34.patch, failed testing.

  • What! Please don't merge the kind of isAvailable() checks in the backend itself, it taints it.
  • Re WHAT! There is no way the ApcBackend actually extends the MemoryBackend, both are very different for different needs: this is a very ugly way of fixing the CLI problem.
  • The APC module does it in a more elegant manner I think: it creates stub implementations when apc_*() functions are missing, which makes it being a null implementation without tainting it with runtime availability checks
  • Agree with Crell, the getPrefix() helper must get out of this specific backend class, it's something than a lot of contrib backends will need, and this should be in a utility class instead or live as a DIC parameter instead to avoid global state
  • All those static:: calls everywhere in all the methods are actually very frightening (and quite ugly)
  • The getAPCKey() and APCBackend sounds like we're yelling I'd personally follow SF for naming and switch to getApcKey() and ApcBackend instead.
  • You forgot the most important point: if I have a multiple frontend installation (multiple boxes with their own webserver for each, but the same site) you can't flush your caches on all frontends at once. I contributed to the APC module, and a colleague of mine and a few other people are actually trying to solve this problem: we cannot seriously support this Apc backend in core without first:
    1. Get in touch with them
    2. Find a generic solution for this problem; because some other backends will need it
  • This patch is clearly NOT YAY at all IMO.

You forgot the most important point: if I have a multiple frontend installation (multiple boxes with their own webserver for each, but the same site) you can't flush your caches on all frontends at once.

You don't even need multiple frontend. The same problem occurs between the command line and the web server.

@pounard:

1) You obviously did not read any issue comments, nor earlier patches, so there's no point in responding to your available/MemoryBackend fallback criticism.

2) Regardless of being injected or not, an externally callable getPrefix() method will be required to figure out the prefix. Even more so, when the prefix needs to be injected. Moving it into a completely separate function outside of the class would only make sense if the identical logic can be applied for other k/v cache backends, which I'm entirely not sure about.

3) PHP uses APCIterator, this is the APCBackend. There's no Symfony involved here. Consistency is thus to be achieved with PHP. In any case, if there's disagreement on that detail » separate issue, thanks.

4) Challenges involving multiple web heads are heavily advanced and can be happily follow-ups to this patch, if needed. Much like the challenge of a possible PHP extension on CLI that's required for the backend, these are actually the secondary major reasons for why I want APC support built-in in core. Alternative implementations for other backends (but also APC) can exist in contrib and learn from the best practices we're figuring out in core.

You obviously did not read any issue comments

Not all that's true. But I did read some of them. Regarding "...but apparently, an APCIterator stub is still missing, and trying to add that in the same way yields" the contrib backend actually don't suffer from it, and in the continuity of the legacy method, we can stub quite easily: class APCIterator extends EmptyIterator {} and it will work seemlessly.

identical logic can be applied for other k/v cache backends

Memcache and Redis are two examples that jumped out of my head in less than 3 seconds after reading that [EDIT: actually Redis was already in my mind since it's a topic in which I'm trying to get uniformisation accross contrib backends because core don't provide anything for it].

PHP uses APCIterator, this is the APCBackend. There's no Symfony involved here. Consistency is thus to be achieved with PHP

We are using Symfony too, both two are valid proposals, I'm just proposing the other one.

can exist in contrib and learn from the best practices we're figuring out in core.

Calm down and respect multiple years of work and many issues solved by many users out there on the existing backend, and don't try to impose what are the "best practices", especially when you cut/pasted lots of code from them.

Challenges involving multiple web heads are heavily advanced and can be happily follow-ups to this patch

Providing an non working implementation as soon as you try to scale, without any warning about incomplete status of the backend is foolish. I don't want non Drupalistas to see that after running their site into prod, after they actually broke it because of it, just because it's "better to do in a follow up" which has 50% chances of never being finished.

In the end, my opinion about this issue is that it is the right one that triggers all those problems and that needs to fix it. The same way that each new use case of the K/V store leads to a K/V store patch in the specific issue, the new problems we discover here must be taken care of right when we found it. Especially when it's a series of long term problems that a lot of people actually are experiencing!

For the record, here are some issues where those problems are trying to be solved:

Regarding CLI vs. webserver (or any other SAPI interface):
A common approach to that seems to be to invoke scripts that require to access the same memory through the desired SAPI; e.g.:

file_get_contents('http://example.com/myscript.php');

There are probably other possible solutions. In any case, I don't really see why that should block us from adding APC support? And why that cannot be discussed in a separate follow-up issue? The problem surely applies to pretty much all alternate caches around, so discussing it on the sole ground of APC would be incomplete anyway.

There are probably other possible solutions. In any case, I don't really see why that should block us from adding APC support? And why that cannot be discussed in a separate follow-up issue? The problem surely applies to pretty much all alternate caches around, so discussing it on the sole ground of APC would be incomplete anyway.

It really only applies to APC (and similar shared memory cache schemes), which is cannot be accessed from outside the web server SAPI. Memcache, Redis, etc. are both cluster-wide and accessible via the CLI.

We cross-posted, I edited my post upper.

We can use the state API to store the latest cache clear timestamp and store its equivalent at flush() call in a key in the APC side. When the object is being built at __construct() time we can compare them and run the flush() if the state() version is higher than the APC local version. This is the most simple solution, and this is probably just something like 10 to 20 lines to implement, so let's do it once for all.

The state() key can actually be set either on a per bin basis, either globally, and can be used by other backends. We can even add a LocalCacheBackendInterface which only adds the getter and setter for the local value, which then leads us to be able to either:
* put the backend behing a generic proxy implementation that does the check
* use the factory/manager to do it for us and leave the backend flush itself at __construct() time.

There is also numerous other smart leads proposed in the given links upper.

Assigned:sun» Unassigned
Status:Needs work» Needs review
StatusFileSize
new9.82 KB
new12.2 KB
FAILED: [[SimpleTest]]: [MySQL] 55,468 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Okay, Let's try to get this issue moving again! I think I agree that the APCBackend should not care about implementing the fallback logic itself. It should be a dumb cache implementation - this should be handled outside of the class.

I have rerolled the patch to remove all this logic from APCBackend, and also update it to the current CacheBackendInterface, as a few things have changed since the last patch. So atm I have just removed the prefix logic, as I think we need somewhere else for it to live.

Status:Needs review» Needs work

The last submitted patch, 1807662-43.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.3 KB
new12.35 KB
FAILED: [[SimpleTest]]: [MySQL] 55,458 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Forgot about $allow_invalid

Status:Needs review» Needs work

The last submitted patch, 1807662-44.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new677 bytes
new12.57 KB
FAILED: [[SimpleTest]]: [MySQL] 55,491 pass(es), 10 fail(s), and 14 exception(s).
[ View ]

Sorry, also needs a deleteTags() method.

Status:Needs review» Needs work

The last submitted patch, 1807662-47.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new12.58 KB
FAILED: [[SimpleTest]]: [MySQL] 57,997 pass(es), 10 fail(s), and 12 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, 1807662-49.patch, failed testing.

APC is less likely to be available now that opcache is in PHP 5.5.

There's http://pecl.php.net/package/APCu which provides the user cache without the opcode caching but not sure how well that's going to be supported.

Indeed. If D8 remains PHP 5.3 compatible, a lot of hostings will still ship APC (think of not-so-bleeding edge Linux distros for example) so I guess having this in core for 8 is still a good idea, while it will probably be removed for later versions. The amount of code needed for this remains minimal and encapsulated, and will be quite easy to maintain for 8.

I agree with pounard on that it's worth it and the good maintainability, plus I don't even expect D9 to rightaway require PHP 5.5, so it would stay in core until D10.

I agree with the above, I think it's a nice little win, and it wont be a big overhead to maintain. I will work on this and get it passing.

Status:Needs work» Needs review
StatusFileSize
new12.93 KB
FAILED: [[SimpleTest]]: [MySQL] 58,202 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

This will get a bit closer.

+++ b/core/lib/Drupal/Core/Cache/APCBackend.phpundefined
@@ -0,0 +1,359 @@
+    apc_store($this->getAPCKey($cid), $cache, $expire);
...
+      apc_inc($this->tagsPrefix . $tag, 1, $success);
+      if (!$success) {
+        apc_store($this->tagsPrefix . $tag, 1);

so result of apc_store() does not matter?

The last submitted patch, 1807662-56.patch, failed testing.

Issue summary:View changes

Updated issue summary.

Title:Built-in APC support in coreBuilt-in APCu support in core (PHP 5.5 only)
Issue summary:View changes

Given that PHP 5.5 has a built-in opcache now, and any D8 site that seriously cares for performance is going to run on PHP 5.5, I'd suggest to limit the scope of this issue to just a built-in Drupal\Core\Cache\ApcuBackend

https://gist.github.com/msonnabaum/9836899 is a quick & dirty hack based on MemoryBackend, but might help