Repository navigation
concat realizes the persistent append its contract declares (normalize O(n·depth) → O(n log n); ampere 147s → 15.5s) - #13461
Merged
Conversation
rc_list_concat and the bare-Vec concat/list_concat extended the left operand one element at a time, so concat cost the length of its right operand against concat_contract's log(n + m). node_subtree_nodes, a bottom-up concat fold called on every parse tree by normalize_with, was therefore O(n * depth): 161 s on one 4,926-row list literal on the emitted compiler. Realize concat as an im::Vector append in runtime_rust.dag and in the interpreter's free concat arms; file the class; correct the build_target annotation that had cleared it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…laim_executor --required-regen, fixed point in 3 rounds) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The v2 native census took hours and looked hung (runs 37348196148 and 37197235156). The cause was concat's realization. It was supposed to be cheap and was not.
What was wrong
std.primitivesconcat_contractdeclares that concat costs log(n + m) on the persistent carrier. The comment next to therc_*ops inv1.compiler.runtime_rustrt_rc_container_opssays the same. But the code did not do that:rc_list_concatranRc::make_mut(&mut a).extend(b.iter().cloned()), pushingbontoaone element at a time.concatandlist_concatused the sameextendpattern.concatarms were worse. They copied both operands into a new vector.So every concat cost the full length of its right operand.
v2.std.nodenode_subtree_nodesis a bottom-up fold whose step isconcat(acc, sub). With that realization its cost was O(n · depth).v2.compiler.normalizenormalize_withcalls it on every parse tree. A comma list parses as a right-nested chain about N deep, so one large list literal was quadratic.perfon the emitted compiler, running census overextdeps.cpu.ampere_altra_package_table4_rawalone, showed:node_subtree_nodes;rc_list_concat;make_mut,drop_slowand Rrbpush_back.The interpreter did not show this. With its call memo off, eval-step counts stayed linear (×2.00 per doubling), because the extra work happened inside a single counted step. Only a profile of the emitted binary named it.
The fix
src/v1/runtime_rust.dag, concat is now an im::Vector append onto a clone ofb, an O(1) structural share that never iteratesb. This coversrc_list_concat, theV2Concatimpl forVec<T>, andlist_concat.rc_list_push,list_pushandlist_snoc_itemwere already fine. Map merge, set union andreverseiterate their input, but they are not list concat, so they are out of scope here.v1_compiler_runtime_rust.rsandv1_rt.rscome from oneclaim_executor --required-regenrun that reached a fixed point in 3 rounds. They were not hand-edited.v1_interpreter.rs, the freeconcatarm and the binary-operator concat path now usevalue_to_list_carrierplus an RRBappend. That is the shape the method andlist_concatarms already had. The copy counters are charged only when a free-monoid chain has to be materialized.gunbc.build_targetaggregate_findingscarried an annotation that checkedrc_list_concatand said it was cheap. It looked at the clone and missed theextendafter it. The annotation now records that error.realization_violates_its_declared_complexity_contract. It is separate fromprimitive_operation_carries_two_cost_authorities: there the declarations disagree, here they agree and the realization contradicts both. The row's next-rung trigger is a scaling control run against the emitted realization of each contracted primitive.Controls
Both sides ran on main
8e95149ecdwith the same script, in separate remote dispatches. The "before" side checks out main'ssrc/v1. Times are for the emitted compiler'scensusover a root containing just that file.ampere_altra_package_table4_raw.dag(4,926 rows, 54k tokens)Semantic equality. I fingerprinted the
normalizeoutput on both sides. The fingerprint is the root'scontent_hash, the node count, and a sum over every node's occurrence identity. Both sides gave identical results:1d3d229b95d4e5ce/5602/37759245before and after.extdeps.ampere.nvparam:e7970b81c12a6cc3/9799/179843194before and after.Out of scope
node_subtree_nodesis called about 200 times across the corpus to build a whole-tree list that each caller then scans. That is linear but redundant demand, and it is tracked as a separate follow-up.Merge
This PR touches
src/v1, so it waits on thesrc/v1hold until #13388 lands.🤖 Generated with Claude Code