Problem/Motivation

When storing bigger items in cache, the memcache module breaks the item down into smaller chunks and then re-combines it before returning it on load. But the code is using a closure function call with array_reduce which creates two performance problems:

1. function calls are expensive/slow in PHP in general, so each call in this example adds another 9ms:
memcache performance issue example

2. as the string that's created is growing and then send to the function it has to be copied and extended in RAM constantly, so each consecutive call gets slower. In this example after the 50th time, an individual call nears 30ms from 9ms at the start:
memcache performance issue decrease over time

Steps to reproduce

Store a large item in cache that has to be chunked e.g. 50 times and profile how long it takes.

Proposed resolution

Replace the array_reduce and inline function with native PHP functions that don't even require a loop like:

return unserialize(implode(array_column($items, 'data')));

https://git.drupalcode.org/project/memcache/-/blob/8.x-2.x/src/MemcacheB...

Remaining tasks

Test and merge

User interface changes

none.

API changes

none.

Data model changes

none.

Issue fork memcache-3386779

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:

Comments

thiemo created an issue. See original summary.

thiemo’s picture

I added a merge request that replaces the function body accordingly: https://git.drupalcode.org/project/memcache/-/merge_requests/16

Please let me know if any changes are required before merging. Thanks!

thiemo’s picture

Status: Active » Needs review

(updating status)

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

LGTM

fgm’s picture

Looks like a no-brainer indeed, but it still would be nice to have a test confirming this:

  • builds a large pseudo-random piece of data
  • purges a bin
  • t0
  • stores the data
  • t1
  • retrieves it
  • t2
  • verify it is found and identical to the original

Run once with the legacy version, once with the new one, and ensure the new one is faster, especially on t2-t1

And run

moshe weitzman’s picture

This project has no test coverage whatsoever. So yes anything “would be nice”

japerry made their first commit to this issue’s fork.

  • japerry committed 63aa2158 on 8.x-2.x authored by thiemo
    Issue #3386779: Memcache performance issue from big (i.e. chunked) cache...
japerry’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.