Problem/Motivation

We've added a static cache layer in #3007091: Performance issue in ConfigurableResourceType, but we are misusing it a bit :)...

Internally the static cache implementations are checking:

// ...
if (!isset($cache->data)) {
  return FALSE;
}
// ...

So when we save NULL this is essentially cache misses on the next cache get for the item.

Before:

After:

Proposed resolution

My proposal is to use FALSE as a tombstone in the cache and have a single ternary operation to fix the return value to a NULL as the interface is promising.

Patch will follow soon.

Remaining tasks

Patch discussion commit.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ndobromirov created an issue. See original summary.

ndobromirov’s picture

Issue summary: View changes
ndobromirov’s picture

Status: Needs work » Needs review
FileSize
1.29 KB

Here is the promised patch...

ndobromirov’s picture

Issue summary: View changes
ndobromirov’s picture

e0ipso’s picture

Status: Needs review » Fixed

Can you please profile the latest version of the -dev branch. It seems that there is a conflict with some recent changes.

#3016725: Do not use cache.static

e0ipso’s picture

Status: Fixed » Needs work
Related issues: +#3016725: Do not use cache.static
ndobromirov’s picture

Will do some time today.

ndobromirov’s picture

Status: Needs work » Needs review
FileSize
1.2 KB
146.49 KB
134.25 KB

Here is the profile with the latest dev:

Here is the one with the micro-optimizations proposed in the other issue and the attached patch:

  • e0ipso committed 1ff3e20 on 8.x-2.x authored by ndobromirov
    Issue #3017262 by ndobromirov, e0ipso: Further speed-ups in...

  • e0ipso committed 0aa1381 on 8.x-3.x authored by ndobromirov
    Issue #3017262 by ndobromirov, e0ipso: Further speed-ups in...
e0ipso’s picture

Status: Needs review » Fixed

Thanks! This was fixed.

ndobromirov’s picture

Awesome!


if I check the first benchmarks 474ms to the last committed ones 22ms - 21x improvement! :D

Status: Fixed » Closed (fixed)

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