InfoParser should use FileCache

This is not yet passing tests.

Also: one question on the MR 🤓

Given that the primary use of FileCache appears to be YAML parsing, wonder if we should add a Yaml::parseCacheableFile() or similar method that wraps the parsing in FileCache and then callers don't need to care about implementing it themselves?

I think we should add a CR just in case anyone is doing something like ThemeUiTest where they are re-writing .info.yml in under a second.

Added the change record.

alexpott → credited kim.pepper → .

Thanks for the review @kim.pepper

I think this is ready. @longwave's #4 has not gotten an answer yet, so letting him re-check/RTBC this, because it sounds like he thinks this should be implemented differently?

I think #4 is follow-up material and I think collections are useful - as shown in the test in this issue where we're able to clear the info parser static cache and make it not use a cache backend.

Also, I think this issue is responsible for a considerable speed up in tests. The webservers are only going to have read every extension info.yml once per job. And as we have APCu enabled in cli same for each CLI process. This is likely to the millions of function calls in each job. See how \Drupal\Core\DrupalKernel::boot() sets the prefix without a site path and we have the apcu_ensure_unique_prefix setting set to FALSE.

Also, I think this issue is responsible for a considerable speed up in tests. The webservers are only going to have read every extension info.yml once per job. And as we have APCu enabled in cli same for each CLI process. This is likely to the millions of function calls in each job. See how \Drupal\Core\DrupalKernel::boot() sets the prefix without a site path and we have the apcu_ensure_unique_prefix setting set to FALSE.

Holy 💩 🤩🤯

You're right!

🤯

Agree #4 can be explored elsewhere.

Discussed in Slack with @alexpott and we think that instead of forcing a cache invalidation in the trait we can just force the mtime of the file to be one second newer than the previous mtime, which should trigger the cache to invalidate by itself.

Did #13 - it looks great - https://git.drupalcode.org/project/drupal/-/merge_requests/6181/diffs?co...

Saving issue credit.

Looks great.

Yes this looks really good. Committed/pushed to 11.x, thanks!

Wow, elegant solution for a test-only scenario! 👏

Please close MR

longwave → closed merge request !6181

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

I know there was some discussion if this needed a change record or not, and ultimately one was added.

I just want to ask that any changes that affect APCu cache always do receive a change record. I help maintain a very large (> 500 sites) multisite install. Any increase to APCu cache utilization, however small, can have cause a big increase in my overall APCu utilization and cause major issues if all allocated memory is exhausted. I don't say this to indicate Drupal shouldn't utilize APCu cache for new things, just that the core committers please ensure that the changes are advertised as change records, just like this one was. It allows me time to test how much more memory I'll need to allocate. Thank you