From 52a29be70d5d91bb4763979837156bcfbcab1df2 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 4 Jun 2015 17:27:31 -0700 Subject: [PATCH] Introduce a request cache mechanism Summary: Ref T8424. This adds a standard KeyValueCache to serve as a request cache. In particular, I need to cache Spaces (they are frequently accessed, sometimes by multiple viewers) but not have them survive longer than the scope of one request. This request cache is explicitly destroyed by each web request and each daemon request. In the very long term, building this kind of construct supports reusing PHP interpreters to run web requests (see some discussion in T2312). Test Plan: - Added and executed unit tests. - Ran every daemon. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T8424 Differential Revision: https://secure.phabricator.com/D13153 --- src/__phutil_library_map__.php | 2 + src/applications/cache/PhabricatorCaches.php | 37 ++++++++++++++++- .../__tests__/PhabricatorCachesTestCase.php | 41 +++++++++++++++++++ .../fact/daemon/PhabricatorFactDaemon.php | 2 + .../PhabricatorRepositoryPullLocalDaemon.php | 1 + .../daemon/bot/PhabricatorBot.php | 2 + .../workers/PhabricatorTaskmasterDaemon.php | 2 + .../workers/PhabricatorTriggerDaemon.php | 2 + webroot/index.php | 2 + 9 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 src/applications/cache/__tests__/PhabricatorCachesTestCase.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 1c6dc98816..02780ee822 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1495,6 +1495,7 @@ phutil_register_library_map(array( 'PhabricatorCacheSpec' => 'applications/cache/spec/PhabricatorCacheSpec.php', 'PhabricatorCacheTTLGarbageCollector' => 'applications/cache/garbagecollector/PhabricatorCacheTTLGarbageCollector.php', 'PhabricatorCaches' => 'applications/cache/PhabricatorCaches.php', + 'PhabricatorCachesTestCase' => 'applications/cache/__tests__/PhabricatorCachesTestCase.php', 'PhabricatorCalendarApplication' => 'applications/calendar/application/PhabricatorCalendarApplication.php', 'PhabricatorCalendarController' => 'applications/calendar/controller/PhabricatorCalendarController.php', 'PhabricatorCalendarDAO' => 'applications/calendar/storage/PhabricatorCalendarDAO.php', @@ -4852,6 +4853,7 @@ phutil_register_library_map(array( 'PhabricatorCacheSetupCheck' => 'PhabricatorSetupCheck', 'PhabricatorCacheSpec' => 'Phobject', 'PhabricatorCacheTTLGarbageCollector' => 'PhabricatorGarbageCollector', + 'PhabricatorCachesTestCase' => 'PhabricatorTestCase', 'PhabricatorCalendarApplication' => 'PhabricatorApplication', 'PhabricatorCalendarController' => 'PhabricatorController', 'PhabricatorCalendarDAO' => 'PhabricatorLiskDAO', diff --git a/src/applications/cache/PhabricatorCaches.php b/src/applications/cache/PhabricatorCaches.php index 1cddba9fb1..e3290172fb 100644 --- a/src/applications/cache/PhabricatorCaches.php +++ b/src/applications/cache/PhabricatorCaches.php @@ -1,12 +1,16 @@ setCaches($caches); } +/* -( Request Cache )------------------------------------------------------ */ -/* -( Local Cache )-------------------------------------------------------- */ + + /** + * Get a request cache stack. + * + * This cache stack is destroyed after each logical request. In particular, + * it is destroyed periodically by the daemons, while `static` caches are + * not. + * + * @return PhutilKeyValueCacheStack Request cache stack. + */ + public static function getRequestCache() { + if (!self::$requestCache) { + self::$requestCache = new PhutilInRequestKeyValueCache(); + } + return self::$requestCache; + } + + + /** + * Destroy the request cache. + * + * This is called at the beginning of each logical request. + * + * @return void + */ + public static function destroyRequestCache() { + self::$requestCache = null; + } + + +/* -( Immutable Cache )---------------------------------------------------- */ /** diff --git a/src/applications/cache/__tests__/PhabricatorCachesTestCase.php b/src/applications/cache/__tests__/PhabricatorCachesTestCase.php new file mode 100644 index 0000000000..7cfa713ead --- /dev/null +++ b/src/applications/cache/__tests__/PhabricatorCachesTestCase.php @@ -0,0 +1,41 @@ +assertEqual( + $default_value, + $cache->getKey($test_key, $default_value)); + + // Set a key, verify it persists. + $cache = PhabricatorCaches::getRequestCache(); + $cache->setKey($test_key, $new_value); + $this->assertEqual( + $new_value, + $cache->getKey($test_key, $default_value)); + + // Refetch the cache, verify it's really a cache. + $cache = PhabricatorCaches::getRequestCache(); + $this->assertEqual( + $new_value, + $cache->getKey($test_key, $default_value)); + + // Destroy the cache. + PhabricatorCaches::destroyRequestCache(); + + // Now, the value should be missing again. + $cache = PhabricatorCaches::getRequestCache(); + $this->assertEqual( + $default_value, + $cache->getKey($test_key, $default_value)); + } + +} diff --git a/src/applications/fact/daemon/PhabricatorFactDaemon.php b/src/applications/fact/daemon/PhabricatorFactDaemon.php index d60a77de6a..384bb56df4 100644 --- a/src/applications/fact/daemon/PhabricatorFactDaemon.php +++ b/src/applications/fact/daemon/PhabricatorFactDaemon.php @@ -9,6 +9,8 @@ final class PhabricatorFactDaemon extends PhabricatorDaemon { protected function run() { $this->setEngines(PhabricatorFactEngine::loadAllEngines()); while (!$this->shouldExit()) { + PhabricatorCaches::destroyRequestCache(); + $iterators = $this->getAllApplicationIterators(); foreach ($iterators as $iterator_name => $iterator) { $this->processIteratorWithCursor($iterator_name, $iterator); diff --git a/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php b/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php index e2895972dd..3ed78f5161 100644 --- a/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php +++ b/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php @@ -73,6 +73,7 @@ final class PhabricatorRepositoryPullLocalDaemon $queue = array(); while (!$this->shouldExit()) { + PhabricatorCaches::destroyRequestCache(); $pullable = $this->loadPullableRepositories($include, $exclude); // If any repositories have the NEEDS_UPDATE flag set, pull them diff --git a/src/infrastructure/daemon/bot/PhabricatorBot.php b/src/infrastructure/daemon/bot/PhabricatorBot.php index b797677777..ce66338111 100644 --- a/src/infrastructure/daemon/bot/PhabricatorBot.php +++ b/src/infrastructure/daemon/bot/PhabricatorBot.php @@ -106,6 +106,8 @@ final class PhabricatorBot extends PhabricatorDaemon { private function runLoop() { do { + PhabricatorCaches::destroyRequestCache(); + $this->stillWorking(); $messages = $this->protocolAdapter->getNextMessages($this->pollFrequency); diff --git a/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php b/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php index 46027814d2..88cc4808cc 100644 --- a/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php +++ b/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php @@ -4,6 +4,8 @@ final class PhabricatorTaskmasterDaemon extends PhabricatorDaemon { protected function run() { do { + PhabricatorCaches::destroyRequestCache(); + $tasks = id(new PhabricatorWorkerLeaseQuery()) ->setLimit(1) ->execute(); diff --git a/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php b/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php index 2c016c3e2c..a8e169b759 100644 --- a/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php +++ b/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php @@ -65,6 +65,8 @@ final class PhabricatorTriggerDaemon $this->nextCollection = PhabricatorTime::getNow(); do { + PhabricatorCaches::destroyRequestCache(); + $lock = PhabricatorGlobalLock::newLock('trigger'); try { diff --git a/webroot/index.php b/webroot/index.php index 6bdcf51478..3e749be0a9 100644 --- a/webroot/index.php +++ b/webroot/index.php @@ -13,6 +13,8 @@ PhabricatorStartup::didStartup(); try { PhabricatorStartup::loadCoreLibraries(); + PhabricatorCaches::destroyRequestCache(); + $sink = new AphrontPHPHTTPSink(); try {