Parents and children of check-in [1f59012870]
|
2026-07-23
| ||
| 10:30 |
delta: bound Apply growth per-command, cut ~5x allocation overhead (#127)
* fix: bound delta.Apply growth per-command and cut ~5x allocation overhead (#81, #82) Two related fixes to the same command loop in delta.Apply: - #82: check len(output) against the declared target length after every copy/insert command, not just at the ';' terminator. A malformed delta with many copy commands could previously grow output well past both targetLen and maxDeltaTargetSize before anything noticed, since the only comparison happened at the end. - #81: once a command has actually executed (proving the delta contains real work, not just an inflated header claim), upgrade output's capacity in a single step sized off len(delta) rather than relying on append's own ~1.25x large-slice growth factor to climb from the 64 KiB initial cap to the final size. That climb was landing near 5x the target size in cumulative allocation; the single upgrade restores near-exact sizing for honest deltas while keeping the tiny-payload/ huge-header attack closed, since a delta with no real commands never reaches the upgrade. Added TestApply_LoopBoundsAgainstTargetDuringCommands (regression for #82: many copy commands exceeding a small declared target are rejected early, with allocation growth bounded near the declared target rather than the full command sequence) and TestApply_LargeTargetAllocationBounded plus BenchmarkApply_LargeTarget (regression/measurement for #81: a 1 MiB target now allocates ~1.06x its size instead of the prior ~5x). Cross-checked against fossil 2.28 for wire-format compatibility; full non-dst suite and dst simulation suite (90 scenarios) pass. * fix: address delta.Apply review nits from #81/#82 Rebuild the #81 regression test/benchmark around a many-small-copy delta shape (4096 copy commands into a 1 MiB target) instead of a Create()-derived single-big-insert delta. The single-insert shape allocates ~1.06x on both fixed and unfixed code (grows once), so it neither exercised the ~5x append-growth-climb regression nor would have caught a future reintroduction of it. Verified against a reverted delta.go: unfixed code now measures ~5.29x on this benchmark, fixed code ~1.06x. Also move the len(output) > targetLen bound checks ahead of their append calls in both the copy and insert command branches, so an over-target command is rejected before its bytes are appended rather than after -- matches the #82 issue wording exactly, closing the (non-exploitable, since a single command's overshoot is already bounded and resident) gap the reviewer flagged as a nit. check-in: 355e7d9910 user: [email protected] tags: trunk | |
| 10:17 |
manifest: visit Crosslink candidates in delta-chain order (#126)
* manifest: create forumpost on demand, matching canonical rebuild Crosslink's candidate query names the forumpost table unconditionally, but canonical fossil creates it lazily -- only once a forum artifact needs it -- and fossil rebuild drops it for a repository that never populated it. Crosslink now recreates the table itself before the sweep runs, matching canonical's on-demand behavior instead of erroring with "no such table: forumpost" against a rebuilt repo. Audited the rest of the on-demand-in-canonical set (ticket, ticketchng) against the current schema: both are created eagerly by canonical at init and survive rebuild, so they don't share this problem. Added TestCrosslinkForumPostTableDropped (drops the table directly) and TestCrosslinkAfterFossilRebuild (drives an actual fossil rebuild first) to cover the regression, and updated the existing fossilbin comparison test's comment now that the rebuild-last ordering is no longer required. * manifest: visit crosslink candidates in delta-chain order Crosslink swept candidates in ascending rid order, but content_deltify stores a blob's older versions as deltas against its newer ones, so delta chains run the opposite direction: high-rid root down to low-rid ancestors. An ascending sweep's first candidate of a chain therefore materializes the whole chain, and the rest are consumed arbitrarily far in the future -- the working set of a whole-repository sweep was the entire repository expanded (8.2 GiB for the Fossil SCM repository). Candidates are now topologically sorted so a delta's base is always visited before its dependents (deltaChainOrder), matching Fossil's own rebuild_step shape: root first, each descendant one delta application from an already-expanded predecessor. The working set this sweep needs is now bounded by how many chains are in flight at once, not by repository size. That reordering also changes the order artifact side effects get written in, since content expansion and artifact crosslinking share one loop. Most of what Crosslink writes turned out to already be a pure function of its own artifact (event/plink/mlink/tagxref-origin rows) and needed no change. Two things generalize across the whole plink graph rather than any one artifact and were genuinely order-sensitive: - leaf table bookkeeping, which inserted the new checkin and deleted its parent -- correct only when parents are linked before their children, which delta-chain order no longer guarantees. - tag propagation, which ran once per checkin from its immediate parent -- reachable only to children plink already held at that moment. Both are now full, order-independent repair passes (repairLeafTable, repairTagPropagation) run once after the sweep, mirroring Fossil's own manifest_crosslink_end + leaf_rebuild() shape: defer the order-sensitive work and repair it once rather than trying to preserve order through the sweep. tag.propagate's mtime-gated, plink-live-at-call-time design turned out to already be confluent -- replaying every self-declared tag origin once, in any order, converges on the same tagxref state -- so the repair is just doing that instead of depending on visiting order to get there implicitly. Tech-note event replacement and forum thread titles turned out to already have the same confluent shape and needed no change; left comments explaining why. Also fixes a tag.PropagateAll crash on any origin with a NULL tagxref value (e.g. CrosslinkCluster's cluster tag), surfaced by the new propagation repair actually reaching those rows for the first time. Regression coverage: a real three-generation checkin chain, forced into a genuine manifest delta chain, that would have left two of three checkins untagged and all three marked leaf under the old per-checkin propagation and incremental leaf bookkeeping. * fix: address review nits on crosslink order-sensitivity and delta cycles - deltaChainOrder now returns an error instead of panicking when the candidate delta graph fails to drain. The delta table is on-disk data that can arrive over sync from an untrusted peer, so a cycle (corrupt or hostile repo) should surface as an error rather than crash the process. - snapshotDerived's digest now covers tagxref (rid/tagid/tagtype/ srcid/origid/value), and TestFossilBinaryReadsCrosslinkedRepo compares it against canonical fossil rebuild -- closing the gap the test previously called out by name ("tagxref is the notable gap"). - Added TestFossilBinaryReadsCrosslinkedRepoBranchAndMerge, exercising the order-sensitive paths incrementalHistory's single-branch history cannot reach: a feature branch diverging from trunk, and a merge commit whose own branch tag must come from its primary parent, not the merged-in one. event/plink/tagxref match canonical fossil row-for-row on this topology. leaf and mlink are deliberately excluded from that comparison: the fixture surfaced that both have pre-existing gaps against canonical fossil (repairLeafTable counts any plink edge rather than only primary-parent edges; insertCheckinMlinks writes a row for every F-card rather than only changed ones) that predate this fix and are out of scope for #102/#103.check-in: 1f59012870 user: [email protected] tags: trunk | |
| 09:52 |
sync: store received wire frames verbatim instead of re-encoding (#132)
* fix: store received wire frames verbatim instead of re-encoding Clone reproduced content correctly but not bytes: the receive path decompressed every cfile frame and re-compressed it with blob.Compress's unparameterized zlib writer, so a clone's blob.content never matched its source's — about 0.5% larger on the wapp corpus, purely from re-encoding frames that were already correctly encoded on the wire. xfer's cfile decoder now hands back StoredBlob alongside the decompressed Content: the same bytes recast into Fossil's on-disk blob format ([4-byte BE size][zlib data]) without a second zlib pass — verbatim when the wire already sent that format (Fossil's send_compressed_file case), or the received zlib stream with a length prefix bolted on when it didn't (our own encoder's plain-zlib case). storeReceivedFile/storeDeltaContent/ storeResolvedContent and blob.StoreDeltaRaw thread that through via a new blob.EncodeForStorage helper, which writes the verbatim bytes when given and only falls back to Compress when there's nothing to preserve — covering both the clone/pull and server-push receive paths. Locally authored content (never received over the wire) always takes the Compress fallback, so #71's deltification policy is untouched. The clone BUGGIFY corruption test drops storedBlob alongside the corrupted content it's exercising, so verbatim storage can't mask the corruption it exists to catch. Adds a corpus-level regression test cloning a real multi-revision fossil repo and asserting sum(length(content)) matches between source and clone. * test: use RequireFossilBin instead of removed HasFossil helper testutil.HasFossil was removed upstream in favor of RequireFossilBin; this test's skip guard needed the same update to compile against current main. check-in: cc42ad8829 user: [email protected] tags: trunk | |