Static cache permissions

Created on 16 September 2014, almost 11 years ago
Updated 6 July 2023, about 2 years ago

Problem/Motivation

PermissionsHandler:moduleProvidesPermissions costs quite a lot and so should be static cached.

Proposed resolution

Static cache result
Let ::getPermissions accept an optional module name.

Remaining tasks

Patch

User interface changes

None

API changes

New optional argument to PermissionsHandlerInterface::getPermissions - BC is maintained

Beta phase evaluation

πŸ“Œ Task
Status

Needs work

Version

11.0 πŸ”₯

Component
User moduleΒ  β†’

Last updated about 6 hours ago

Created by

πŸ‡©πŸ‡ͺGermany dawehner

Live updates comments and jobs are added and updated live.
  • Performance

    It affects performance. It is often combined with the Needs profiling tag.

  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡¦πŸ‡ΊAustralia almunnings Melbourne, πŸ‡¦πŸ‡Ί

    With patch
    Drupal 10.1 gets a heap of weird issues on installing modules.
    Specifically with Drupal\Component\DependencyInjection\ReverseContainer

    Service Container has a null value for generateServiceIdHash.

    Throws a delightfully difficult error to trace.

    TypeError: Drupal\Component\DependencyInjection\ReverseContainer::generateServiceIdHash(): Argument #1 ($object) must be of type object, null given in /app/site/web/core/lib/Drupal/Component/DependencyInjection/ReverseContainer.php on line 87 #0 [internal function]: Drupal\Component\DependencyInjection\ReverseContainer->generateServiceIdHash(NULL)
    #1 /app/site/web/core/lib/Drupal/Component/DependencyInjection/ReverseContainer.php(75): array_map(Array, Array)
    #2 /app/site/web/core/lib/Drupal/Core/DrupalKernel.php(903): Drupal\Component\DependencyInjection\ReverseContainer->recordContainer()
    #3 /app/site/vendor/drush/drush/src/Drupal/DrupalKernelTrait.php(70): Drupal\Core\DrupalKernel->initializeContainer()
    #4 /app/site/web/core/lib/Drupal/Core/DrupalKernel.php(816): Drush\Drupal\DrupalKernel->initializeContainer()
    #5 /app/site/web/core/lib/Drupal/Core/Extension/ModuleInstaller.php(608): Drupal\Core\DrupalKernel->updateModules(Array, Array)
    #6 /app/site/web/core/lib/Drupal/Core/Extension/ModuleInstaller.php(244): Drupal\Core\Extension\ModuleInstaller->updateKernel(Array)
    #7 /app/site/web/core/lib/Drupal/Core/ProxyClass/Extension/ModuleInstaller.php(83): Drupal\Core\Extension\ModuleInstaller->install(Array, false)
    #8 /app/site/web/core/lib/Drupal/Core/Config/ConfigImporter.php(872): Drupal\Core\ProxyClass\Extension\ModuleInstaller->install(Array, false)
    #9 /app/site/web/core/lib/Drupal/Core/Config/ConfigImporter.php(624): Drupal\Core\Config\ConfigImporter->processExtension('module', 'install', 'graphql_compose...')
    #10 /app/site/web/core/lib/Drupal/Core/Config/ConfigImporter.php(561): Drupal\Core\Config\ConfigImporter->processExtensions(Array)
    #11 /app/site/vendor/drush/drush/src/Drupal/Commands/config/ConfigImportCommands.php(300): Drupal\Core\Config\ConfigImporter->doSyncStep('processExtensio...', Array)
    #12 /app/site/vendor/drush/drush/includes/drush.inc(122): Drush\Drupal\Commands\config\ConfigImportCommands->doImport(Object(Drupal\Core\Config\StorageComparer))
    #13 /app/site/vendor/drush/drush/includes/drush.inc(113): drush_call_user_func_array(Array, Array)
    #14 /app/site/vendor/drush/drush/src/Drupal/Commands/config/ConfigImportCommands.php(271): drush_op(Array, Object(Drupal\Core\Config\StorageComparer))
    #15 [internal function]: Drush\Drupal\Commands\config\ConfigImportCommands->import(Array)
    #16 /app/site/vendor/consolidation/annotated-command/src/CommandProcessor.php(276): call_user_func_array(Array, Array)
    #17 /app/site/vendor/consolidation/annotated-command/src/CommandProcessor.php(212): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback(Array, Object(Consolidation\AnnotatedCommand\CommandData))
    #18 /app/site/vendor/consolidation/annotated-command/src/CommandProcessor.php(176): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter(Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
    #19 /app/site/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(391): Consolidation\AnnotatedCommand\CommandProcessor->process(Object(Symfony\Component\Console\Output\ConsoleOutput), Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
    #20 /app/site/vendor/symfony/console/Command/Command.php(326): Consolidation\AnnotatedCommand\AnnotatedCommand->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
    #21 /app/site/vendor/symfony/console/Application.php(1081): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
    #22 /app/site/vendor/symfony/console/Application.php(320): Symfony\Component\Console\Application->doRunCommand(Object(Consolidation\AnnotatedCommand\AnnotatedCommand), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
    #23 /app/site/vendor/symfony/console/Application.php(174): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
    #24 /app/site/vendor/drush/drush/src/Runtime/Runtime.php(124): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
    #25 /app/site/vendor/drush/drush/src/Runtime/Runtime.php(51): Drush\Runtime\Runtime->doRun(Array, Object(Symfony\Component\Console\Output\ConsoleOutput))
    #26 /app/site/vendor/drush/drush/drush.php(79): Drush\Runtime\Runtime->run(Array)
    #27 /app/site/vendor/drush/drush/drush(4): require('/app/site/vendo...')
    #28 {main}
    TypeError: Drupal\Component\DependencyInjection\ReverseContainer::generateServiceIdHash(): Argument #1 ($object) must be of type object, null given in Drupal\Component\DependencyInjection\ReverseContainer->generateServiceIdHash() (line 87 of /app/site/web/core/lib/Drupal/Component/DependencyInjection/ReverseContainer.php).
    
  • πŸ‡ΊπŸ‡ΈUnited States mglaman WI, USA

    This shouldn't use a cache property. It should use a memory cache backend that falls back to a permanent cache backend.

    Like JSON:API does:

      # Cache.
      cache.jsonapi_memory:
        class: Drupal\Core\Cache\MemoryCache\MemoryCache
        public: false
      cache.jsonapi_resource_types:
        class: Drupal\Core\Cache\BackendChain
        calls:
          - [appendBackend, ['@cache.jsonapi_memory']]
          - [appendBackend, ['@cache.default']]
        tags: [{ name: cache.bin }]
    

    This allows for invalidating the in-memory cache within one request.

  • πŸ‡¬πŸ‡§United Kingdom catch

    Just read down and was thinking the same as #59, and then there was the comment so I didn't have to write it :)

    Although given #41 I think we should add the memory backend here and then move persistent caching, if it's needed at all, to a follow-up.

  • πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia

    kim.pepper β†’ made their first commit to this issue’s fork.

  • Status changed to Needs review over 1 year ago
  • πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia

    Pushed an MR that adds a MemoryCache.

  • Pipeline finished with Failed
    over 1 year ago
    Total: 1137s
    #96353
  • Status changed to Needs work over 1 year ago
  • πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia

    Lots of test fails for this. I assume we need to work out where the cache needs to be cleared.

  • πŸ‡¬πŸ‡§United Kingdom jonathanshaw Stroud, UK
  • πŸ‡ΊπŸ‡ΈUnited States mglaman WI, USA

    I just ran into this again. I had a Recipe that applied 30 permissions. It took 2 seconds because the role validation causing `\Drupal\user\PermissionHandler::buildPermissionsYaml` to run at each granted permission. I'm guessing \Drupal\Core\Extension\ModuleInstaller::doInstall would need to reset cache on each install.

    Cache tag isn't great since Drupal de-dupes multiple invalidations. But then the module handler is aware of the cache ID. Unless PermissionHandler has a specific "reset" method added.

  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    > Cache tag isn't great since Drupal de-dupes multiple invalidations.

    Yes, but only if there were no new caches set since the last invalidation, that's not an issue.

    However, cache tags still need to be actually invalidated, and there are _many_ dynamic permissions based on bundles and other config entities, all those would suddenly need to add cache tag invalidations.

    This is going to require a lot of work and it might be easier to deal with specific issues on a higher level. For example, the recipe action could support a list of permissions instead of saving the role config entity 30 times, that's always going to be slower.

Production build 0.71.5 2024