Page MenuHomePhabricator

ParserCache should store all used options
Closed, ResolvedPublic

Description

Currently the first tier of the ParserCache only stores used options which are in the cache key and dismisses others. This creates two problems:

  1. If we want to add more options into the cache key, we do not know if the pages have used them so it's extremely hard to transition
  2. ParserOutput with non-canonical non-in-cache-key options is considered unsafe to cache now, so the cache is bypassed, even if these options were not actually used in the parse. Storing all used options will allow us to check whether cache can be used more precisely potentially improving the hit rate.

Event Timeline

Change 644957 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Record all used options in metadata.

https://gerrit.wikimedia.org/r/644957

Not opposed to this, but I don't quite see why it's needed: For all ParserOutput options currently in the cache, we know that any option that was not recorded did not influence the output. So we can safely assume that it was not used.

Not opposed to this, but I don't quite see why it's needed: For all ParserOutput options currently in the cache, we know that any option that was not recorded did not influence the output. So we can safely assume that it was not used.

Not exactly. Right now we only actually record the options which are 1) were used 2) are included in the cache key. So for any other option, the ones not in the cache key, they are not recorded. So, when we are checking if PArserOptions are safe to cache or not, we are checking if they have a non-standard value for ANY non-cache-key option, regardless of whether it was used or not. It's a small optimization.

So, when we are checking if PArserOptions are safe to cache or not, we are checking if they have a non-standard value for ANY non-cache-key option, regardless of whether it was used or not. It's a small optimization.

Right, I see - if the option is unsafe but wasn't used, we can still cache. I wonder how often that happens though.

Change 649719 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Respect used options for ParserOptions::isSafeToCache

https://gerrit.wikimedia.org/r/649719

Change 644957 merged by jenkins-bot:
[mediawiki/core@master] Record all used options in metadata.

https://gerrit.wikimedia.org/r/644957

Change 649719 merged by jenkins-bot:
[mediawiki/core@master] Respect used options for ParserOptions::isSafeToCache

https://gerrit.wikimedia.org/r/649719

Change 667823 had a related patch set uploaded (by Reedy; owner: Ppchelko):
[mediawiki/core@REL1_35] Record all used options in metadata.

https://gerrit.wikimedia.org/r/667823

Change 667823 merged by jenkins-bot:
[mediawiki/core@REL1_35] Record all used options in metadata.

https://gerrit.wikimedia.org/r/667823