Skip to content

input: one compiled link regex per pattern for the whole process - #69

Merged
lawrencecchen merged 2 commits into
release/cmux-9ce9b7d0-e2from
perf/link-regex-once
Nov 3, 2026
Merged

lawrencecchen merged 2 commits into
release/cmux-9ce9b7d0-e2from
perf/link-regex-once

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Each new surface compiled every configured link regex twice on the main thread: once for the surface (Surface.DerivedConfig.init) and once for its renderer (renderer/link.zig Set.fromConfig). In a cmux workspace switch trace (Time Profiler, 6 switches, cmux run 20261103T031956Z-ed80d47d) that was 6.3 ms of onig_compile per switch, about 0.5 ms per compile of the default URL regex.
  • New src/input/link_regex.zig: one compiled regex for each pattern in the process, counted by its users. Surfaces and renderer link sets acquire on init and release on deinit. Up to 16 unused patterns stay compiled, so a switch that frees surfaces and then makes new ones compiles nothing. Matching does not change (same options as input.Link.oniRegex).
  • Thread safety: Oniguruma 6.9.9 onig_search never writes to a regex_t (no reg-> assignment in regexec.c; the last search-time write, int_map_backward, was removed "for thread-safe", HISTORY 2016/07/17). Only compile and free change a regex_t, and both happen under the cache lock.
  • Base: release/cmux-9ce9b7d0-e2 (cmux's pin 34e73ba8b), so cmux can pin it without the v1.4.0 / Zig 0.17 merge on main. A port to main is a follow-up.

Testing

  • Red (test commit 5410e5d44, run 41574025483 on scratch branch scratch/tcc-link-regex-red, which only narrows the test filters): 2 of 2 new tests fail ("expected struct_re_pattern_buffer@...030, found ...230"); 82 other tests pass.
  • Green (fix dfd07de4a, run 41574028017 on scratch/tcc-link-regex-green, which adds the new filters to the full patch filter list): ghostty-test 222/222, lib-vt 475/475, build, no-gettext and link smoke pass.
  • The workflow's test filter list is not changed in this PR (no new CI test step). The scratch branches are never merged.

Changelog

none


Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.


Summary by cubic

Shares one compiled link regex per configured pattern across all surfaces and renderers, so a new surface no longer compiles each regex twice on the main thread (about 0.5 ms per URL regex compile, 6.3 ms of onig_compile per workspace switch in cmux's trace).

  • Adds src/input/link_regex.zig: a process-wide cache of compiled regexes, reference-counted by surfaces and renderer link sets (acquire on init, release on deinit).
  • Patterns unused for a while stay compiled (up to 16), so a workspace switch that frees surfaces and creates new ones compiles nothing.
  • Sharing is safe across threads: Oniguruma 6.9.9 search never writes to a regex_t; only compile and free mutate it, and both happen under the cache lock. Matching behavior is unchanged.
  • Bases off release/cmux-9ce9b7d0-e2 so cmux can pin it without the v1.4.0 / Zig 0.17 merge; porting to main is a follow-up.

Written for commit dfd07de. Summary will update on new commits.

View guided diff Turn on auto-fix

Every new surface compiled each configured link regex twice on the main
thread (Surface DerivedConfig and the renderer link.Set). In cmux's
workspace switch trace this was 6.3 ms of onig_compile per switch
(about 0.5 ms per new surface). These tests fail until the compiled
regexes are shared.
Every new surface compiled each configured link regex twice on the main
thread: once for the surface (DerivedConfig) and once for its renderer
(link.Set). With the default URL regex that is about 0.5 ms per compile;
cmux's workspace switch trace showed 6.3 ms of onig_compile per switch.

input/link_regex.zig keeps one compiled regex per pattern, counted by its
users (surfaces and renderer link sets acquire on init, release on
deinit). Up to 16 unused patterns stay compiled, so a switch that frees
surfaces and makes new ones compiles nothing. Oniguruma 6.9.9 search
never writes to a regex_t, so one regex serves the app and renderer
threads of every surface. Matching is unchanged.
@coderabbitai

coderabbitai Bot commented Nov 3, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: efcd0b86-5808-4fe5-a52d-1607dfdb3314

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

@lawrencecchen
lawrencecchen merged commit dfd07de into release/cmux-9ce9b7d0-e2 Nov 3, 2026
4 checks passed
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (independent review subagent): SHIP at dfd07de

Reviewed 34e73ba8b..dfd07de4a (5410e5d44 red tests, dfd07de4a fix). The acquire/release pairs replace the old compile/free pairs 1:1 in Surface DerivedConfig and renderer link.Set. The renderer config ownership paths are unchanged (initChangeConfig, Message.deinit, changeConfig, renderer deinit). Only entries with 0 refs are evicted, and the cache mutex is a leaf lock.

The oniguruma 6.9.9 regexec.c check confirms the claim. No reg-> field is assigned during a search. The only write is the direct-threaded opaddr fill when msa == NULL, and that runs only at compile time (onig_init_for_match_at, regcomp.c:7637). onig_search uses a per-call match param. The key is the pattern bytes only, which is correct because the options are fixed.

Release xcframework-dfd07de4a...-apple-v6 (manifest commit dfd07de4a, SHA256SUMS a04d2fc3...) matches the cmux pin in manaflow-ai/cmux#32997.

Nits: a partial failure in the acquire loops leaks earlier refs (an existing pattern, harmless). The assert(entry.refs > 0) is UB in ReleaseFast if a release is ever unbalanced. The new tests are not in the workflow filter list.

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.

1 participant