perf: destroy only the arena nodes that own something (0118 P5b-1)

[rmorozov]

🤖 Generated with Claude Code

https://claude.ai/code/session_01QT4pkibQA9uD34K5vYkC2j

Before: every arena node has a virtual destructor and an entry in the sealed tree's cleanup table. Seal destroys every original node after moving it, and the template's destruction runs every node's destructor. ValueRefExpression's destructor also touches the thread's lookup cache (LookupCache::Forget).

After: a node that owns nothing is trivially destructible and is never destroyed. Seal and the template's destruction walk a cleanup table of the owners only. Behaviour is unchanged.

This is the first P5b PR (wave 2, plan approved by the perf track). The next ones make the template root, raw text and the remaining names and lists own nothing, after which every non-object node is trivial and a static_assert enforces it.

How:

• Destructors. Every leaf node class is final. ExpressionEvaluatorBase, IRendererBase, Statement, SetStatement, SetBlockStatement and ExpressionRenderer have protected non-virtual destructors. The three bases that still own members and have subclasses (ValueRefExpression, SubscriptExpression, MacroStatement) keep public virtual destructors until P5b-3.
• The trait. IsTriviallyDestructibleNode (node_arena.h) probes a protected destructor through a final subclass. It gives a null destroy op, and Make counts the owners.
• Seal. MoveNodes writes the owners' offsets into the cleanup table at the end of the sealed buffer. It throws logic_error if Make counted fewer owners than the ops say. Seal's second pass and SealedArena::DestroyNodes walk only that table.
• Bitmaps. The node-start bitmap, and FULL's validated bitmap, now live in the sealed buffer, so ArenaView::Checked is a bit test instead of a binary search over the cleanup table, and Seal no longer allocates a heap bitmap for large trees. With NODEREF_CHECKS=OFF there is no bitmap, and Checked skips the object-start test. That check used to run even when OFF; the doc comment already said "at every level but OFF".
• Lookup cache. LookupCache::Forget and ~ValueRefExpression are deleted. An entry hits only under the epoch it was cached in, every render, context copy and clone takes a new epoch, and a render keeps every tree it runs alive. A tree loaded at a freed tree's addresses is therefore never seen under the freed one's epoch. A Debug assertion checks that a context is used only on the thread whose cache it holds (the perf track accepted this instead of globally unique epochs, which cost Render +0.34..+0.51%). The invariant is documented at LookupCache. The escaped-callable constraint is in the design plan's Escapes section.
• Inlining fix. FullExpressionEvaluator::Render calls the base Render out of line (RenderEvaluated). With final, inlining it gave the fast path an InternalValue frame (Render/expressions +0.36%).
• Tests.
  • SealKeepsCleanupOnlyForOwners.
  • LookupCacheIgnoresAFreedTemplatesEntries, and the same across threads.
  • Helpers.LookupCacheEntriesEndWithTheirEpoch replaces the two Forget tests.
  • Helpers.LookupCacheIsPerThread.
  • A static_assert lists the node kinds that must stay trivial.

Numbers (bench/count.py, Release, NODEREF_CHECKS=ON, vs 4fca3f9 = master + 0146)

Load case	Instructions	Change	Retained (cap)
plain_text	4,456	-1.09%	752 (848)
substitute	11,287	-0.99%	944 (1,088)
for_range	21,692	-0.32%	1,432 (1,648)
inheritance	38,324	-0.57%	1,960 (2,568)
many_tags	13,785,832	-0.79%	543,040 (-4,720); peak -12 KB, one allocation fewer
other Load cases		-0.2..-0.75%

Render: every case within ±0.1%.

Most node kinds still own a name, a constant or a vector until the next P5b PRs. That is why this PR's saving is small.

Process

• Roles: an architect produced the P5b design and inventory; the main session implemented and measured; one verifier round.
• Verifier, round 1, two blocking findings, both fixed before the first push:
  • clang's -Wabstract-final-class on the trait's probe of the abstract filter and tester interfaces. Classes with a virtual destructor now skip the probe.
  • clang-tidy hits: protected = delete members in Statement, the trait's value naming, and namespace-scope statics in the test.
• Verifier, clean results: Debug FULL, Release shared and NODEREF=OFF builds; ASan+UBSan and TSan with clang; and 24 malformed templates against Python Jinja2. It found no Seal memory-safety issue and no path that breaks the lookup-cache argument.
• Red pushes: 1. The first push failed the CodeQL check with 3 "suspicious add with sizeof" alerts on std::byte* + idx * sizeof(T) in OffsetTable and TestSealedBit. The arithmetic was correct (byte pointers), but the second commit names the byte offset first, which clears the alert.

---

Generated by Claude Code