Twig cache timestamp is written but never read

Seems to have a test failure. Also possible that contrib module could be using it?

Should it be NW for that?

The timestamp is read when auto_reload is on. Twig's own test defines the semantics: with getTimestamp() returning 0 the loader is never asked and write() is called, so the template is recompiled and rewritten on every request. See EnvironmentTest::testAutoReloadCacheMiss.

default.services.yml recommends enabling auto_reload rather than disabling the cache, so that path is a documented setup.

An alternative is to read the timestamp from the compiled file. MTimeProtectedFastFileStorage::save() already sets the containing directory mtime to the file mtime, so the value is there. The cache writes still go away and the interface keeps returning a real timestamp.

Measured on current main with only the class change: CacheSetCount in StandardPerformanceTest goes from 42 to 32, and those ten cids are all twig:. The other expectation changes in the MR do not reproduce here.

I have a patch with kernel test coverage, and can open a follow-up if you prefer the plain removal here.

Thanks for picking this one up, and sorry for the slow reply.

In #4 you mentioned you could not reproduce the test failure locally. I had a go at it, and I think the good news is that the failure is not coming from your change.

Your own pipeline shows the same thing. Job https://git.drupalcode.org/project/drupal/-/jobs/11177159 on pipeline 902253 fails in testStandardPerformance, on the $expected_queries assertion inside testAnonymous(), with Failed asserting that two arrays are identical. Four queries are in the expected array and never turn up in the recorded one:

SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "system.maintenance_mode" ) AND "collection" = "state"
SELECT "name", "value" FROM "key_value" WHERE "name" IN ( "system.private_key" ) AND "collection" = "state"
INSERT INTO "semaphore" ("name", "value", "expire") VALUES ("state:Drupal\Core\Cache\CacheCollector", "LOCK_ID", "EXPIRE")
DELETE FROM "semaphore"  WHERE ("name" = "state:Drupal\Core\Cache\CacheCollector") AND ("value" = "LOCK_ID")

They all come from the State cache collector. They clearly run on your machine, and they do not run on the runners or here. I could not work out what makes the difference, so that part is over to you: you know your setup far better than I can guess at it.

I then ran three variants on main ef161e27d8, to see which of the expectation changes reproduce outside your machine.

Your branch as it is fails on $expected_queries, the same way your CI does.

Your code change, with the test file taken from main, passes $expected_queries happily. The only failure left is CacheSetCount: 42 expected, 32 recorded.

Reading the timestamp from the mtime of the compiled file, with the test from main and CacheSetCount set to 32, passes with 52 assertions.

So CacheSetCount 42 to 32 looks like the only expectation change in the MR that reproduces outside your machine. main still expects QueryCount => 10.

One more thing, in case it saves you a look: PHPUnit Unit (Component): [8.6-ubuntu] is green on current main. It failed on 902253, but that base d97ade0537 (26 July) predates 0439c440a2 (29 July), which is the PHP 8.6 fix for FileStorageTest from #3613160: PHP 8.6 failure in FileStorageTest. So that one is not yours either.

Environment here: DDEV, PHP 8.5, MariaDB 10.11, selenium standalone-chrome:133.0.

On the approach, I pushed a variant to 3613278-timestamp-from-mtime on this issue fork, with kernel test coverage, in case it is useful. It reads the timestamp from the mtime of the compiled file rather than returning 0, so the cache writes still go away and getTimestamp() keeps returning a real value. Your branch is untouched.

Related: #2888082: Deploying Twig template changes is too expensive: it requires all caches to be completely invalidated, as well as all reverse proxies covers the same ground on the deployment side.

What would you like me to do from here? I am happy to open a merge request for it, push to your branch, or leave the approach as it is and just drop the $expected_queries change so your MR can go green. Just let me know which one helps, or if something else would be more useful.

@sjpagan thanks for contributing but just a few notes.

Based on previous posts, not just here but on several others, it seems like AI has been heavily used so please take a moment and read https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett....

If AI is used it needs to be disclosed and written in your own words. Phrase like "Thanks for picking this one up, and sorry for the slow reply." when the previous comment was from yourself make it seems like it was just copy and paste from Claude. Instead you should read the output and write it yourself.

There's no rule against using AI at all but again please read the issue etiquette page.

Thanks.

Pushed a commit restoring the testAnonymous() expectations to main.
CacheSetCount 42 to 32 stays.

Pipeline 915363 is green. Functional Javascript 1/3 passes. The two failing
PHP 8.6 jobs allow failure; Unit (Component) is #3613160, which landed after this branch's base.

I also warmed node/1 before the cache clear to see if that changed anything, and no metric moved. The user page I left alone.

If you put the cache write back, StandardPerformanceTest fails on its own, so the removal already has coverage.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.