Skip to content

perf: InternalValue access seam in wave 2's files, plain string alternatives (0140 P0b) - #460

Merged
rmorozov merged 3 commits into
masterfrom
claude/project-thread-xwi8pm
Oct 11, 2026
Merged

rmorozov merged 3 commits into
masterfrom
claude/project-thread-xwi8pm

Conversation

@rmorozov

Copy link
Copy Markdown
Member

Requested by Ruslan · project thread

Before: the evaluator and statements reached into InternalValue's variant directly (GetData(), GetIf<MapAdapter>, holds_alternative), and strings were stored as nested variants (TargetString, TargetStringView), so every Apply checked three wrappers before visiting.

After: code outside internal_value.* and value_visitors.h reads values only through the P0a seam (AsMap, AsStringView, Kind(), GetIf for callables/pairs), and a value holds std::string, std::wstring, std::string_view or std::wstring_view itself. Rendering output is unchanged; Render instructions drop 0..3.5%.

This is step P0b of docs/design/0140-internal-value-plan.md: after it, P2's 32 B layout has call sites only in internal_value.* and value_visitors.h.

How.

  • expression_evaluator.cpp/.h, statements.cpp: GetIf<MapAdapter> → AsMap (0154's x.name fast path included), GetStringView → AsStringView<char>, IsImmutableScalar and InlineScalar::Holds use Kind() (+ new IsStringView).
  • NarrowStringView (python_format, serialize_filters callers) and GetStringView (markup.h) removed.
  • GetData() replaced by detail::InternalValueAccess, used only in internal_value.* and value_visitors.h; unused Get<T> removed.
  • InternalValueData alternatives: TargetString/TargetStringView → the four plain string types; InternalValue has constructors from TargetString/TargetStringView that unpack; ApplyUnwrapped unwraps only host values.

Deviation from the plan. The plan expected 72 → 64 B here. It stays 72 B because ListAdapter is also 40 B (accessor, two flags and m_fieldNames from #401). P2 moves lists behind the object pointer, so no interim packing was done; recorded in docs/tasks/0140.

Instruction counts (local, bench/count.py vs 1ca9c15). Render: many_tags -3.50%, for_loop_vars -2.90%, strings -2.35%, for_filter_if -2.07%, macros -1.94%, html_autoescape -1.71%, expressions -1.50%, filters -1.40%, dict_ops -1.12%, large_static -0.94% (21,403, already under P2's 21,472 gate), chat_qwen -0.06%, chat_llama -0.19%, inheritance -0.32% (all three P2 gates back at or under base), for_range +0.16%, rest within ±0.6%. Load: within ±0.9% (expressions +0.84%, filters -0.84%).

Checks. Full test suite, parity corpus (incl. wide) and fuzz regressions; ASan+UBSan (clang-18) clean; 205 templates rendered byte-identically to the base; scripts/preflight.sh --perf all PASS.

Agent roles. Main session implemented; verifier 1 round (ready, no blocking findings). Red CI pushes: none yet.

🤖 Generated with Claude Code

https://claude.ai/code/session_015qGBQQVdqpanbWPSsaMQrL


Generated by Claude Code

…natives (0140 P0b)

expression_evaluator.*, statements.cpp, python_format and serialize_filters
go through AsMap, AsStringView and Kind(); GetStringView and
NarrowStringView are removed. GetData() is replaced by
detail::InternalValueAccess, used only by internal_value.* and
value_visitors.h. TargetString/TargetStringView stop being variant
alternatives: values hold std::string, std::wstring, std::string_view or
std::wstring_view directly, and ApplyUnwrapped unwraps only host values.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015qGBQQVdqpanbWPSsaMQrL
…tor (clang-tidy)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015qGBQQVdqpanbWPSsaMQrL
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015qGBQQVdqpanbWPSsaMQrL
@rmorozov rmorozov self-assigned this Oct 11, 2026
@rmorozov
rmorozov marked this pull request as ready for review October 11, 2026 12:57
@rmorozov
rmorozov merged commit 6471f17 into master Oct 11, 2026
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants