Problem/Motivation

Every GET /node/{node}/advanced-comment-threads/{field_name}/refresh does the following:

  • loads every comment on the field;
  • runs access('view') and subject field access on each one;
  • builds the whole payload;
  • only then hashes it for the ETag.

References: src/Controller/CommentFragmentController.php:249-293, src/CommentIndexRepository.php:156-248 and src/Element/CommentHistory.php:47-82.
A 304 saves bandwidth but no server work. Phase 0 (#3623896) made the cacheability merge linear and removed baseline/newCommentIds from the refresh payload, so anonymous 304s now happen. Measured on Drupal 11.4.6, PHP 8.4, MariaDB 11.4, with database cache bins and a warm entity cache:

  • 1,000 comments: about 0.4 s → 0.3 s per poll.
  • 10,000 comments: about 14 s → 3.8 s per poll, still about +340 MB peak memory, which is more than a common 256 MB memory_limit.

An offline benchmark of a snapshot hit plus the per-user overlay, JSON encoding and hashing: about 1 ms at 1,000 comments and about 10 ms at 10,000.

Proposed resolution

Implement ADR 0009 phase 1 (docs/adr/0009-shared-refresh-snapshot-cache.md, "Decision" and "Plan"):

  • Snapshot builder service. It produces a scalar-only snapshot per host, field, sort direction, published_only and access variant: orderedIds, parentById, childCounts, versions, pending ids, created times, owners whose uid is visible, and the host author's ids. It loads entities in chunks (about 200), resets entity.memory_cache between chunks and keeps no entities.
  • Storage. A dedicated bin (for example cache.act_thread) through VariationCache. Do not use ChainedFast: every comment write would flush the APCu layer for the whole bin. The initial cacheability is user.permissions. The final cacheability is what the access results bubbled, minus the snapshot's own comment:N tags, plus node:N, the ACT settings config tag and one host tag, act_comments:{entity_type}:{entity_id}.
  • Invalidation. Invalidate the host tag from the existing ACT hooks in src/Hook/CommentHooks.php:85-111 (insert, update, delete). Invalidate both the old and the new host when an update moves a comment. The delete hook is required: core's Comment::postDelete() does not invalidate the host.
  • Staleness rules. Do not store when the final max-age is 0. Otherwise apply a safety TTL of about 15 minutes.
  • Rebuilds. Rebuild under a lock with a short wait (1–2 s). After that, fall back to a last-good copy at most a few seconds old, or build without storing. Never hold PHP workers for a whole large rebuild.
  • Kill switch. advanced_comment_threads.settings:refresh_snapshot_cache, on by default, with a schema entry and a post update.
  • Refresh. refresh() and CommentHistory::lazyBuilder() take snapshot plus overlay (own/right-aligned ids, viewer signature, visit token, acknowledge URL). The ETag stays a SHA-256 of the JSON actually sent (ADR 0004).

Remaining tasks

  • Implement the service, bin, invalidation, lock/fallback and kill switch.
  • Kernel tests:
    • hit, miss and invalidation on insert, update, move and delete;
    • max-age: 0 access results are not stored;
    • per-user variants only when an access result adds user;
    • anonymous responses are byte-identical across visitors.
  • Update the test access hooks in tests/modules/advanced_comment_threads_formatter_test that read state without a tag or max-age: 0. Under Drupal's access contract they must declare one (see ADR 0009 "Effects").
  • Re-run the 1k/10k measurements; record hit latency and miss memory.
  • Update docs/loading-strategies.md "Known limitations" (the first bullet, around line 658).

User interface changes

None.

API changes

  • New service and cache bin, both @internal.
  • New config key refresh_snapshot_cache.
  • The refresh payload shape is unchanged.

Data model changes

New config key only.

Comments

freelock created an issue. See original summary.

freelock’s picture

Issue summary: View changes

  • freelock committed 1a3bc7ff on 1.0.x
    perf: #3626381 Drop the unused lock from the fragment controller
    
    The...
freelock’s picture

Issue summary: View changes
Status: Active » Fixed

Done in Beta1.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.