Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enh: New RuntimeCache #6482

Draft
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

martin-rueegg
Copy link
Contributor

@martin-rueegg martin-rueegg commented Aug 4, 2023

A new RuntimeCache (RTC) interface and implementation that offer the following ...

Functionality

  • Store one and the same value at different keys (alias, links)
  • Update, delete such a distinct value at once for all keys
  • Derive key names (or aliases) automatically from the to-be-stored value, if it's an object and implements \yii\db\ActiveRecordInterface or \humhub\interfaces\UniqueIdentifiersInterface. This allows that e.g. a User is automatically stored (and hence can be retrieved) both by its id or guid.
  • This makes easy deletion of an object by simply using the object as the key for deletion.
  • Define exclusion rules to avoid storage of certain keys. If an object has multiple keys, it is not stored if one single key is excluded.
  • Define inclusion rules to fine-grain the exclusion functionality (e.g. don't cache User objects, generally, but do cache the current user)
  • Define maximum of stored entries in order to keep memory footprint low. The the least-read values are discarded first, unless it's a new entry which has a grace period/number

Possible improvements

  • It could be useful do define exclusions for class names. Currently this is only possible if the class allows for automatically derived keys, in which case this key can be matched against reliably.
  • Save some diagnostic statistics somewhere to know what values for exclusion and limits should be used
  • Observe memory consumption and base limits on that
  • Keep timestamp of last read and include that in the prioritizing process during garbage collection

Known issues

  • Since testing is done without any cache (see also Codeception Tests with enabled Caching #6462), this cannot be reliably tested through automatic testing (as neither was or can the original implementation of PR Performance Improvements #6375). Simply activating cache during tests would probably not do the trick, since
    • Cache would have to be pruned between tests to ensure reproducibility
    • Given the above, real-life situations might differ significantly enough from testing scenarios in the sense that e.g. loading a list of posts form all the same person or many different persons would cause the user(s) to be cached (or not) in the RTC.
    • Although - since the RTC has a life-time of ony one request - the Cests could adequately test the RTC, tests would have to be run twice, once with and once without RTC, since the RTC can disabled in the config.

Requirements

This PR is based on and requires the changes of PR #6457 and PR #6463. (The changes from this PR alone are to be seen in this commit: 8df0534)

Breaking changes

1. Interface requirements for RuntimeCache

Currently, since the introduction of PR #6375, any \yii\caching\CacheInterface can be used for the RuntimeCache. However, now the \humhub\interfaces\RuntimeCacheInterface is required.

2. Unique identifier pattern changed

Applies to tests (e.g. CommentCest.php), potentially to some front-end code?

Since the code for ActiveRecord::getUniqueId has changed and is no longer removing the backslashes (\), but replacing them with unterlines (_), the resulting identifiers change in the front-end. However, the identifier is now used both for front-end as well as for caching, making it a unique and coherent pattern.

PR Admin

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Refactor

The PR fulfills these requirements:

  • It's submitted to the develop branch, not the master branch if no hotfix
  • When resolving a specific issue, it's referenced in the PR's description (e.g. Fix #xxx[,#xxx], where "xxx" is the Github issue number)
  • All tests are passing
  • New/updated tests are included
  • Changelog was modified

@martin-rueegg martin-rueegg changed the title Enh new RuntimeCache Enh: New RuntimeCache Aug 4, 2023
@martin-rueegg martin-rueegg force-pushed the enh/new-runtimeCache branch 2 times, most recently from ad9fadf to 4e66957 Compare August 4, 2023 13:56
@martin-rueegg martin-rueegg marked this pull request as ready for review August 4, 2023 15:14
@martin-rueegg martin-rueegg force-pushed the enh/new-runtimeCache branch 5 times, most recently from 9fc337d to 8df0534 Compare August 11, 2023 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant