Problem/Motivation

ChainedFast is a nice backend, which has it's problems as it needs to write back all data again and again to the fast backend.

One of the most CPU intensive things is the constant serialization and unserialization -- even if it's usually handled by the cache backends themselves.

One of the reasons is that Drupal uses DEEP arrays, and DEEP objects.

However if you do:

$item = (object) [
  'data' => serialize($data),
];
$store = serialize($data);

this is almost as fast as:

$item = (object) [
  'data' => $data,
];
$store = serialize($data);

because the one level for serialization is not responsible for a huge performance penalty.

And we can use this to our advantage!

Proposed resolution

If we serialize() the data before set() and unserialize() the data after get() / getMultiple(), then in practice it means that 90% of the data is already serialized, which we get back from the consistent backend!

Which means we save 90% of the serialization effort when saving data to the fast backend!

And that in the end means that things are 90% faster.

(And APCu is storing serialized data as well and no longer copying raw PHP objects into memory, so that's no longer a concern.)

Also: Because we do not need to unserialize the data coming from APCu, we can effectively save the unserialize() step when the data is invalid -- before we would unserialize twice.

Remaining tasks

- Create patch based on the following:

diff --git a/web/core/lib/Drupal/Core/Cache/ChainedFastBackend.php b/web/core/lib/Drupal/Core/Cache/ChainedFastBackend.php
--- a/web/core/lib/Drupal/Core/Cache/ChainedFastBackend.php
+++ b/web/core/lib/Drupal/Core/Cache/ChainedFastBackend.php
@@ -170,6 +170,10 @@ public function getMultiple(&$cids, $allow_invalid = FALSE) {
       }
     }
 
+    foreach ($cache as $cid => $item) {
+      $item->data = unserialize($item->data);
+    }
+
     return $cache;
   }
 
@@ -177,6 +181,8 @@ public function getMultiple(&$cids, $allow_invalid = FALSE) {
    * {@inheritdoc}
    */
   public function set($cid, $data, $expire = Cache::PERMANENT, array $tags = []) {
+    $data = serialize($data);
+
     $this->consistentBackend->set($cid, $data, $expire, $tags);
     $this->markAsOutdated();
     // Don't write the cache tags to the fast backend as any cache tag
@@ -188,6 +194,10 @@ public function set($cid, $data, $expire = Cache::PERMANENT, array $tags = []) {
    * {@inheritdoc}
    */
   public function setMultiple(array $items) {
+    foreach ($items as &$item) {
+      $item['data'] = serialize($item['data']);
+    }
+
     $this->consistentBackend->setMultiple($items);
     $this->markAsOutdated();
     // Don't write the cache tags to the fast backend as any cache tag

User interface changes

- None

API changes

- None

Data model changes

- None

CommentFileSizeAuthor
#34 3014521-34.patch2.13 KBqusai taha
#29 Screenshot 2023-01-22 at 14-01-04 Blackfire.io Continuous Performance Testing.png32.78 KBberdir
#26 3014521-26.patch2.15 KBit-cru
#21 3014521-21.patch1.26 KB_utsavsharma
#21 interdiff_19-21.txt493 bytes_utsavsharma
#19 3014521-19.patch1.27 KBit-cru
#11 3014521-11.patch1.27 KBsivaji_ganesh_jojodae
#6 3014521-6.patch1.27 KBzahord
#3 apcu_store_chainedfast_90_faster.png135.53 KBfabianx

Issue fork drupal-3014521

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Fabianx created an issue. See original summary.

fabianx’s picture

fabianx commented
Issue summary: View changes
fabianx’s picture

fabianx commented
Issue summary: View changes
StatusFileSize
new apcu_store_chainedfast_90_faster.png135.53 KB
fabianx’s picture

fabianx commented
Issue summary: View changes
andypost’s picture

andypost
he/him
Primary language Russian
commented

What if replace this call with configurable serializer so igbinary (for example) could be used here

zahord’s picture

zahord commented
StatusFileSize
new 3014521-6.patch1.27 KB

Hi! created patch file using copy code shared

zahord’s picture

zahord commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 6: 3014521-6.patch, failed testing. View results

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.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.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

sivaji_ganesh_jojodae’s picture

sivaji_ganesh_jojodae commented
Status: Needs work » Needs review
StatusFileSize
new 3014521-11.patch1.27 KB

The patch still applies and the test result shows success on the "View Results" page.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Issue tags: -Novice, -needs patch +Needs tests

I am removing the Novice tag from this issue because I am not sure it is clear what the next step is. The current patch and approach failed tests.

Also I think that having an additional test or looking to see how this change behaves with how existing data is serialized.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

it-cru’s picture

it-cru
Primary language English
Location Munich
commented
StatusFileSize
new 3014521-19.patch1.27 KB

Provided patch for D10.1.x

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Needs work

The static analysis errors need fixing.

_utsavsharma’s picture

_utsavsharma commented
StatusFileSize
new interdiff_19-21.txt493 bytes
new 3014521-21.patch1.26 KB

Tried to fix CCF for #19.

_utsavsharma’s picture

_utsavsharma commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 21: 3014521-21.patch, failed testing. View results

socialnicheguru’s picture

socialnicheguru commented

Could http://drupal.org/project/igbinary be used for a similar result for Drupal 8,9?

it-cru’s picture

it-cru
Primary language English
Location Munich
commented

@SocialNicheGuru maybe it could be configurable at cache.backend.chainedfast service definition. Especially when someone doesn't use DB and APCu as chainedfast caching backend. I think this also depends on how often igbinary PHP extension is enabled by default.

Maybe at the first step it would be good to get some feedback, that this is already such a performance improvement and it doesn't break something for other caching setups for example with Memcached, Redis and so on.

In my setup I run it together with Memcached as default caching backend with following config with default service config for cache.backend.chainedfast from core (currently only tested locally in my DDEV setup).

    $settings['cache']['bins']['bootstrap'] = 'cache.backend.memcache';
    $settings['cache']['bins']['discovery'] = 'cache.backend.chainedfast';
    $settings['cache']['bins']['config'] = 'cache.backend.chainedfast';

    // Use memcache as the default bin.
    $settings['cache']['default'] = 'cache.backend.memcache';
it-cru’s picture

it-cru
Primary language English
Location Munich
commented
Status: Needs work » Needs review
StatusFileSize
new 3014521-26.patch2.15 KB

Based on patch from #21 and adjusted ChainedFast related tests.

catch’s picture

catch
he/him
Primary language English
commented

igbinary has an existing issue: #1340260: Support for igbinary.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Priority: Major » Normal
Issue tags: -Needs tests

I'm a bit confused by the patch as it seems to ignore the conditional serialization that the databasebackend and most others do, although it also says at the same time that serializing a string is very efficient, which I assume is quite possible.

I did some profiling by forcing the fast chained backend to always treat the fast one as dirty. apcu_store() took about 12.5ms (2.85% of the full request as admin with disabled render cache on umami) and blackfire is no longer reporting those calls at all then with the patch.

That said, the title is clearly misleading, it's only 90% faster *if* we have a dirty fast backend (and with #3334489: ChainedFastBackend invalidates all items when cache tags are invalidated, that should be way, way less frequent) and it's less only about 2-3% speedup of the whole request, so I'm downgrading that from major to normal.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Title: Improve performance of ChainedFast by 90% » Improve fast-backend write performance of ChainedFastBackend by serializing upfront
Issue tags: +Needs issue summary update
StatusFileSize
new Screenshot 2023-01-22 at 14-01-04 Blackfire.io Continuous Performance Testing.png32.78 KB

To add to that, even when just looking at the isolated performance of ChainedFastBackend, an all-miss situation looks like this:

So writing back the cache entries only takes 25% of the time, so we save ~90% of 25%.

Here's an attempt at a better issue title that reflects what it is actually about.

One thing to keep in mind is also that this change is quite tricky to apply on a site, adding or removing the patch or updating if it were to be committed with any existing caches is not be handled and probably should be.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative

#29 requested an issue summary update so moving to NW for that.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

IT-Cru opened merge request !8524

it-cru’s picture

it-cru
Primary language English
Location Munich
commented

Rollback of patch from #26 as MR to apply against 11.x and 10.3.x again.

One kernel and some functional unit tests are failing in Gitlab CI jobs now, but I have no clue why this wasn't previously the case. So I think this needs some more investigations.

qusai taha’s picture

qusai taha commented
StatusFileSize
new 3014521-34.patch2.13 KB

Re-roll the patch to make it working with 10.3.2

socialnicheguru’s picture

socialnicheguru commented

I was migrating a site from Drupal 10 to Drupal 10.3.

I applied the patch in composer.json:
"Improve fast-backend write performance of ChainedFastBackend by serializing upfront":"https://www.drupal.org/files/issues/2024-09-01/3014521-34.patch",

I received the following error:

Drush command terminated abnormally due to an unrecoverable error. Error: Uncaught TypeError: unserialize(): Argument #1 ($data) must be of type string, array given in drupal-10.3.x/html/core/lib/Drupal/Core/Cache/ChainedFastBackend.php:174 Stack trace:
#0 drupal-10.3.x/html/core/lib/Drupal/Core/Cache/ChainedFastBackend.php(174): unserialize()
#1 drupal-10.3.x/html/core/lib/Drupal/Core/Config/CachedStorage.php(87): Drupal\Core\Cache\ChainedFastBackend->getMultiple()
#2 drupal-10.3.x/html/core/lib/Drupal/Core/Config/ConfigFactory.php(165): Drupal\Core\Config\CachedStorage->readMultiple()
#3 drupal-10.3.x/html/core/lib/Drupal/Core/Config/ConfigFactory.php(104): Drupal\Core\Config\ConfigFactory->doLoadMultiple()
#4 drupal-10.3.x/html/core/lib/Drupal/Core/Config/ConfigFactory.php(89): Drupal\Core\Config\ConfigFactory->doGet()
#5 drupal-10.3.x/html/core/lib/Drupal.php(414): Drupal\Core\Config\ConfigFactory->get()
#6 drupal-10.3.x/html/core/includes/errors.inc(323): Drupal::config()
#7 drupal-10.3.x/html/core/includes/errors.inc(123): _drupal_get_error_level()
#8 drupal-10.3.x/html/core/includes/bootstrap.inc(204): error_displayable()
#9 drupal-10.3.x/html/core/includes/bootstrap.inc(188): _drupal_exception_handler_additional()
#10 [internal function]: _drupal_exception_handler()
#11 {main} thrown in drupal-10.3.x/html/core/lib/Drupal/Core/Cache/ChainedFastBackend.php, line 174

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.