kaizen: Embed smalltable

[sayrer]

This one carries #521 further, fixes some bugs that caused buildshellstyle quadratic behavior, and also speeds up Nfa2dfa.

[timbray]

(afk for a while, will take a closer look)

[timbray]

OK, checked out 535 and ran the research-main CSV generator. I'll attach the xlsx files and a graph of sec/100-addP-calls

[2026-05-31.xlsx](https://github.com/user-attachments/files/28443966/2026-05-31.xlsx) [2026-05-30.xlsx](https://github.com/user-attachments/files/28443967/2026-05-30.xlsx)

[timbray]

Now, granted, all these AddPattern calls are wildcards, and the epsilon merging gets kind of ugly, so probably not a representative workload.

[timbray]

I'm just trying to figure out whether I care about this. Don't have an opinion yet.

[sayrer]

I just got that workload, seems like I can do some damage to it with some small changes.

[timbray]

I gotta say, if those numbers are correct, the permanent memory saving is just not that dramatic and the AddPattern impact is pretty significant. So maybe the thing to do is back out the last PR and then cherrypick the good stuff from the faState/smallTable branch with the add-ons that made nfa2Dfa faster and so on?

[sayrer]

I think I can get it with no regression, just measuring.

[sayrer]

Even just the first step went well. The closure walk was improved, and had been going over the more space-efficient structures too much.

That's here:
5fe237b

  Incremental epsilon closure — results

  Plan executed (commits on embed-smalltable-research): spec 5fe237b → plan
  534327a → immutability gate 18547f6 → order-independence guard a6c44a9 →
  prune 5976a4f → review nits f96e39a (plus 20c980d research -cpuprofile
  flag).

  The win (10k-pattern research harness):

  ┌──────────────────────┬──────────────┬─────────────────────────────────┐
  │                      │ before prune │           after prune           │
  ├──────────────────────┼──────────────┼─────────────────────────────────┤
  │ patterns/sec         │ 133–140      │ 418 (3×)                        │
  ├──────────────────────┼──────────────┼─────────────────────────────────┤
  │ wall time            │ ~72 s        │ 24.6 s                          │
  ├──────────────────────┼──────────────┼─────────────────────────────────┤
  │ build-scale @ N=1000 │ 473 ms       │ 225 ms (~2.1×)                  │
  ├──────────────────────┼──────────────┼─────────────────────────────────┤
  │ closureForNfa flat   │ 8.46 s       │ 0.06 s (walk re-traversal gone) │
  └──────────────────────┴──────────────┴─────────────────────────────────┘

[sayrer]

So, this is already getting close on AddPattern, and the footprint improvements improve the Matcher.

[timbray]

Is this PR up-to-date with your latest?

[sayrer]

No, this is on https://github.com/sayrer/quamina/tree/embed-smalltable-research, which has everything in this PR, your research PR, and further patches to see if the AddPattern cost is really necessary. I didn't want to pile them all in to one review, but I did want to see whether it's possible to avoid any unpleasant trade-offs, and that's looking promising.

[timbray]

Oh, ok, I'll pull that and have a look around tomorrow sometime.

[timbray]

OK, this PR now stable from your POV?

[sayrer]

yes

[timbray]

There are three distinct things in here:

1. Mostly have things start on faState rather than smallTable, including the field in valueMatcher. I have wanted to do this for like a year, so this is great. I think this job perhaps isn't 100% done (see comments on arguments to mergeFAs and valueMatcher generally) but probably don't need to block on that. I may be able to go back and drastically simplify {"exists":true} matching depending simply on whether vm.start is nil or not.
2. Turn state.table from *smallTable to smallTable. Excellent.
3. Refactor epsilonClosure. Frankly I don't understand what's happening here and there are a lot of fields to support it. I've suggested adding a large overview comment, probably sourced from your Claude dialog. I'd prefer not to add the Claude stuff in docs/ to the project as a thing in itself.

Anyhow, this is huge and almost ready, thanks!

[timbray]

I'll be interested in what the profiler says. This used to dominate nfa2Dfa performance…

[sayrer]

Profiled BenchmarkNfa2Dfa (M5, CPU profile). Intern still dominates nfa2Dfa, ~65% of its CPU (1.94s of 3.0s cum).

But the cost has moved. After this branch's changes (sort+compact dedup, reusable scratch buffers, PutUint64 key-build, alloc-free string(keyBuf) lookup), the marshalling is now cheap and the hot line is the map lookup itself:

┌───────────────────────────────┬───────────────────────┬─────────────────────────┐
│             line              │         cost          │          what           │
├───────────────────────────────┼───────────────────────┼─────────────────────────┤
│ sl.entries[string(sl.keyBuf)] │ 1.08s — 56% of intern │ string-hash + map probe │
├───────────────────────────────┼───────────────────────┼─────────────────────────┤
│ slices.SortFunc               │ 280ms                 │ canonical-key sort      │
├───────────────────────────────┼───────────────────────┼─────────────────────────┤
│ append(sortBuf[:0], list...)  │ 250ms                 │ copy input to scratch   │
├───────────────────────────────┼───────────────────────┼─────────────────────────┤
│ key-build loop (PutUint64)    │ 130ms                 │ per-state               │
├───────────────────────────────┼───────────────────────┼─────────────────────────┤
│ rest (compact / miss alloc)   │ ~140ms                │                         │
└───────────────────────────────┴───────────────────────┴─────────────────────────┘

So the code around the lookup is about as tight as it gets; the residual is the irreducible set-dedup hash. Further wins would need a cheaper key/hash or fewer intern calls, not trimming the surrounding code. (The big madvise/GC in the raw profile is n2dNode building DFA states across nfa2Dfa, not intern.)

[sayrer]

OK, I think I got everything or explained them at least.

[timbray]

I'll go through this a little later, but a quick note on first glance: If you go through quamina.AddPattern() you can't have a spinner on the start state, because regexps can only apply to strings, so the first step in the regexp is always a literal " or 0x22. In testing, it's easy to build a wildcard or regexp that starts with * or .*, but not in actual use.

Arguably Quamina always retaining the enclosing quotations adds 2 extra steps to FA traversal, but it simplifies a bunch of other things.

[timbray]

Only remaining quibble is the sync.Pool() stuff. Also see earlier note about root spinners (can't happen)

[timbray]

You mean "independent matcher builds" on an entirely separate Quamina instance? In the same Go program? That seems wildly unlikely. As the comment makes clear, you can't have concurrent matcher builds on the same instance.

I still can't see why it wouldn't be simpler & probably faster just to have a single set of closureBuffers around per instance and have a resetClosureBuffers() call to clear the maps at the start of AddPattern. There's no reason ever to GC this stuff unless you know for sure you're never going to call AddPattern again.

[timbray]

Now reading https://pkg.go.dev/sync#Pool and trying to figure out what I think.

[sayrer]

I'll go through this a little later, but a quick note on first glance: If you go through quamina.AddPattern() you can't have a spinner on the start state, because regexps can only apply to strings, so the first step in the regexp is always a literal " or 0x22. In testing, it's easy to build a wildcard or regexp that starts with * or .*, but not in actual use.

Arguably Quamina always retaining the enclosing quotations adds 2 extra steps to FA traversal, but it simplifies a bunch of other things.

I wrote a buggy test that starts with *. will fix.

[timbray]

The more I think about it, the less comfortable I get with sync.Pool. Apparently it randomly ejects things from cache in GC operations. But our pattern is super-deterministic. For each AddPattern, we

1. Clear the maps in the cache
2. add lots of stuff and consult it frequently while we build automata and epsilon closures
3. Are done with it (could clear it here)

I'm not 100% sure what the effect is of syncPool() deciding to expire stuff while we're at stage 2 in the list above, but it's not going to be helpful.

Could we take out sync.Pool() and just have a simple use/clear rhythm?

[sayrer]

Could we take out sync.Pool() and just have a simple use/clear rhythm?

  - Full suite green; -race clean on the Copy()/concurrent-AddPattern paths 
    (the key risk of dropping the concurrency-safe pool).

  - Build +22% vs the pool (732 vs 599 patterns/sec, clean repeated harness runs) — and
    notably the naïve never-clear version was −7%, so the per-AddPattern clear() is what
    makes it a win (keeps the scratch maps holding one build's working set instead of
    growing to O(all states).

  - Match path untouched; match benchmarks unchanged.
  
  - Generation counters retained (they avoid clearing within a build;
    reset() bounds size across builds).

[timbray]

This is about the fifth time I've observed sync.Pool being more expensive than seems reasonable.

Heh, Claude needs to really internalize the Quamina concurrency model. "-race clean" is not a surprise.

Anyhow, this looks good, I'll do one more sweep but I think we can merge.

One question: GitHub is whining about an unsigned commit. Is this an admin error you remember? if so, no problem. Otherwise, maybe an attacker?

[timbray]

This has become da big one, happy to land it.

At some future point it may be worth trying to simplify epsi_closure.go which is now maybe the most complicated piece of logic in the whole project. But it'll do for now.

[timbray]

At some future point I'm going to rename this back to newFA but first I'm going to merge this.

[sayrer]

One question: GitHub is whining about an unsigned commit. Is this an admin error you remember? if so, no problem. Otherwise, maybe an attacker?

It's a new laptop (no Migration Assistant) and I forgot to do the ssh key. It's fixed now, but once one commit slips in, it won't verify the following ones in a PR.

[timbray]

Got it, thanks. Merging…