Draft: Load rapid diffs metadata without the patch text

Stacked on !256363. The stored-file path for diffs_stats and diff_files_metadata still selected every row's diff column and built diff objects holding the patch, only so the collection could count lines and bytes for its overflow flags, and diff_files_metadata then loaded every blob through GetBlobs just to read the SHA for code_review_id. This selects the counts in SQL, never loads the patch text for these two endpoints, and resolves all blob ids with one FindChangedPaths RPC. Same per-file, overflow and code_review_id results, verified against the full path.

Rollout order across the epic (&23646): 1. gitaly!9274 (merged) (no flag, Gitaly release + gem and GITALY_SERVER_VERSION bump), then !256363 and its stacked !256402 (shared flag rapid_diffs_metadata_from_stored_diff_files). !256379 (merged) (flag diff_skip_redundant_charset_detection) and !256367 (closed) (flag diff_file_blob_id_metadata_only) are independent and can merge any time. With every flag off, behaviour is unchanged apart from the Feature.enabled? call. This MR: stacked on !256363, same flag, stays Draft until its base merges.

Detailed context for AI agents

Why the patch was loaded at all. Neither endpoint reads patch content. The generic Gitlab::Git::DiffCollection needs, per stored row: the binary check in expand_diff? (now skipped by !256379 (merged)), Gitlab::Git::Diff#line_count and #diff_bytesize for collapsed? / too_large? and for the collection's cumulative collapsed_safe_* / overflow flags, plus Gitlab::Diff::File#added_lines falling back to parsing the patch for renamed files. Profiling a 231-file MR: selecting and decoding 587 KB of diff text plus building the objects was most of the remaining wall time once charset scans were gone.

Change.

  • MergeRequestDiffFile.without_patch_text: selects every column except diff, plus COALESCE(octet_length(diff), 0) AS patch_bytesize and a patch_line_count expression that reproduces Gitlab::Git::Util.count_lines (newline count, +1 when the text does not end in a newline, 0 for empty). Rows with binary (base64-encoded, only decoding gives the real size) or renamed_file (line counts parsed from the patch because DiffStats has no rename detection, see aec05fb1) keep their diff via CASE WHEN "binary" OR renamed_file THEN diff END.
  • MergeRequestDiffFile#to_hash: when patch_bytesize is present and the row is neither binary nor renamed, emits diff: '' plus diff_bytesize and line_count.
  • Gitlab::Git::Diff#init_from_hash: a hash carrying diff_bytesize sets @diff = '' and pre-fills the memoized @diff_bytesize / @line_count, so too_large?, collapsed?, prune_diff_if_eligible and line_count behave exactly as with the text. prune! also zeroes @diff_bytesize, matching the emptied text.
  • DiffCollection#each_serialized_patch accumulates diff.diff_bytesize instead of diff.diff.bytesize (identical for full patches, since pruning zeroes both).
  • MergeRequestDiff#load_diffs applies without_patch_text when the caller passes metadata_only: true and the diff is not stored_externally? (external rows only know external_diff_size, no line count, so they keep the current path).
  • MergeRequestDiff#diffs_metadata now uses the stored rows whenever files are stored and the flag is on, not only when whitespace is ignored, and passes metadata_only: true, use_extra_viewer_as_main: false (the .ipynb renderable check would otherwise parse the empty patch).
  • RapidDiffs::DiffsStatsEntity reads the overflow flags from raw_diff_files; diff_files on the merge request collection also hmgets the highlight cache for every file, which the stats never used.

Blob ids without blob loads. code_review_id hashes file_identifier with a blob id. Stored rows carry no blob ids (to_id/from_id are not in SERIALIZE_KEYS), so Gitlab::Diff::File#blob_id fell through to blob&.id: one GetBlobs per 250 files at a 512 KB content limit, plus a second batch for the old blobs of deleted files. On the 1000-file MR that was five sequential RPCs, about 160 ms, two thirds of the request. Persisting the ids is not an option: merge_request_diff_files is in OverLimitTables and Migration/PreventAddingColumns blocks add_column on it. Instead, when metadata_only is set, FileCollection::MergeRequestDiffBase#blob_ids_resolver hands each Gitlab::Diff::File a lazy callable that runs one FindChangedPaths on the diff refs (diff_refs || fallback_diff_refs, the same refs new_content_sha / old_content_sha use) and maps path => { old:, new: }, with blank refs mapped to nil. blob_id prefers to_id, from_id, then the preloaded id, then the blob lookup, so a missing path or a Gitaly error just falls back. It is lazy, so diffs_stats, which never asks for code_review_id, still makes zero Gitaly calls. Ids verified identical to blob&.id for every file of the four test MRs and in the spec. No caching, by request.

Equivalence checks. On GDK for MRs of 43, 231, 268 and 1000 files: to_hash sizes equal diff.bytesize and count_lines(diff) for every row (0 mismatches); per-file collapsed?, too_large?, line_count, new_file?, deleted_file?, renamed_file? and the collection's collapsed_safe_*, overflow?, real_size are identical between diffs and diffs_metadata. spec/models/merge_request_diff_spec.rb asserts the same per-file and overflow state for the fixture MR, in all three storage variants of the shared examples (DB, external always, external outdated).

Measured on GDK (median of 8, flag on, warm stats cache, both whitespace modes give the same numbers):

files diffs_stats before (!256363) diffs_stats now diff_files_metadata before (!256363) diff_files_metadata now
43 29 ms 11 ms 45 ms 40 ms
231 165 ms 25 ms 204 ms 57 ms
268 104 ms 28 ms 193 ms 63 ms
1000 163 ms 89 ms 372 ms 144 ms

diffs_stats makes zero Gitaly calls. diff_files_metadata makes exactly one, FindChangedPaths, instead of one to five GetBlobs. The original live CommitDiff path measured 70 / 126 / 131 / 258 ms for diff_files_metadata on the same MRs, so the stored path is now faster at every size in both whitespace modes.

Rename note. DiffStats already detects renames at git's default 50% similarity and reports them with old_path; CommitDiff uses 30%. Renames between the two thresholds are the only case where the counts differ, which is why renamed rows keep their patch here and why Gitlab::Diff::File#added_lines parses them. Lowering the threshold was considered (gitaly#7428 (closed)) and closed as not needed.

Specs run (all green): spec/models/merge_request_diff_file_spec.rb, spec/lib/gitlab/diff/file_spec.rb, spec/models/merge_request_diff_spec.rb (#diffs_metadata, all storage variants), spec/lib/gitlab/git/diff_spec.rb, spec/lib/gitlab/git/diff_collection_spec.rb, spec/lib/gitlab/diff/file_collection/, spec/serializers/rapid_diffs/diffs_stats_entity_spec.rb, spec/presenters/rapid_diffs/merge_request_presenter_spec.rb, and the GET #diff_files_metadata / GET #diffs_stats request specs.

Consolidated findings (2026-09-18, epic &23646)

Where the 500s came from. Both diffs_stats and diff_files_metadata fail inside Gitlab::GitalyClient::DiffStitcher while iterating a CommitDiff stream. An MR only takes that path when whitespace changes are hidden (DiffHelper#hide_whitespace? is true for every anonymous user and for users with show_whitespace_in_diffs off), when the diff files were cleaned from the DB (without_files), or for a commit_id. Gitlab::Git::Compare resolves both SHAs first, so the failing status is not a missing commit; the class is in the Sentry title (ResourceExhaustedError is already rescued globally).

What CommitDiff really streams. Gitaly runs git diff-tree -p over the whole range and its parser reads every patch byte; with whitespace hidden it runs a second git diff --numstat over the whole diff for line counts (gitaly!8539 (merged)). Patch bodies are sent in full up to the safe limits (100 files, 5000 lines, 500 KB), pruned to metadata past them, and cut off at the hard limits (1000 files, 50 000 lines, 5 MB). For MRs under 100 files, which is most, the two endpoints streamed and re-parsed the complete diff to read a handful of counts. Neither endpoint reads a patch byte: stats needs totals, count, real_size and overflow flags; metadata needs paths, flags, hashes, a blob id and per-file counts.

Why nobody had done this. #588880 (closed), gitaly#7067 (closed), gitaly#7086 (closed), gitaly!8539 (merged) and !223439 (merged) deliberately moved line counts into CommitDiff so the stream could drop DiffStats, on the assumption that the patch is fetched anyway. That holds for the stream and never held for these two endpoints. MergeRequestPresenter#include_diff_stats? still returned true for MRs, so DiffStats was never removed for them either.

Stored path costs found on the way. (1) Two CharlockHolmes charset scans per stored patch, in DiffCollection#expand_diff? and Gitlab::Git::Diff#encode_diff_to_utf8: 55% of wall time plus 28% GC on a 231-file MR, both skippable without changing results (!256379 (merged)). (2) The diff column was selected for every row only to count lines and bytes for the overflow flags; SQL can return those counts (!256402). (3) code_review_id needs a blob id, stored rows carry none, so every request ran GetBlobs in batches of 250 with a 512 KB content limit, 5 RPCs and two thirds of the request on a 1000-file MR. Persisting to_id/from_id is blocked: merge_request_diff_files is in OverLimitTables and Migration/PreventAddingColumns rejects add_column. One FindChangedPaths RPC returns identical ids for every path in 25 to 45 ms (!256402). The metadata-only GetBlobs variant (!256367 (closed)) was closed as redundant: Rapid Diffs is fully enabled and it only helped the legacy diffs page.

Renames. DiffStats already detects renames at git's default 50% similarity and reports old_path; CommitDiff uses --find-renames=30%. Only renames between the two thresholds differ, which is what the unless renamed_file? fallback in Gitlab::Diff::File#added_lines covers, so renamed rows keep their patch and gitaly#7428 (closed) was closed (50% is acceptable).

Rollout order and flags. gitaly!9274 (merged) first (no flag; needs a Gitaly release plus gitaly gem and GITALY_SERVER_VERSION bump). Then !256363, then !256402 stacked on it, both behind rapid_diffs_metadata_from_stored_diff_files (rollout #629875), Draft until the Gitaly release ships and the flag must stay off until that Gitaly is deployed, because an older Gitaly ignores the unknown proto field and returns whitespace-inclusive counts. !256379 (merged) is independent behind diff_skip_redundant_charset_detection (rollout #629894). With every flag off, behaviour is unchanged apart from the Feature.enabled? call.

Retiming with everything on (GDK, stack plus !256379 (merged), flags off vs on in the same process, medians of 8; stream = collection load without rendering):

files whitespace stats off / on metadata off / on stream load off / on page total off / on
43 shown 50 / 13 ms 63 / 40 ms 74 / 54 ms 186 / 107 ms (43%)
43 hidden 63 / 12 ms 65 / 34 ms 116 / 82 ms 244 / 128 ms (47%)
231 shown 191 / 29 ms 211 / 58 ms 320 / 171 ms 722 / 257 ms (64%)
231 hidden 108 / 27 ms 110 / 54 ms 321 / 142 ms 540 / 223 ms (59%)
268 shown 139 / 35 ms 230 / 65 ms 603 / 519 ms 972 / 619 ms (36%)
268 hidden 130 / 32 ms 132 / 65 ms 278 / 162 ms 539 / 259 ms (52%)
1000 shown 192 / 95 ms 337 / 155 ms 493 / 396 ms 1022 / 646 ms (37%)
1000 hidden 236 / 95 ms 266 / 161 ms 666 / 539 ms 1168 / 795 ms (32%)

Gitaly RPCs per page load with whitespace hidden: 12 down to 7 (stats FindCommit x2 + CommitDiff to none; metadata FindCommit x2 + CommitDiff to one FindChangedPaths; the stream keeps its 6 because it needs the patches). With whitespace shown, stats and metadata make zero and one RPC instead of DiffStats plus up to five GetBlobs batches. The charset skip also speeds up the whitespace-hidden stream by 19 to 56%, since Gitaly patches go through the same second scan. First request per diff version with flags on pays one DiffStats (15 to 60 ms), cached for a week. Totals and per-file flags were verified identical to the old path on all four MRs.

Still open, in the epic. #629872 (Compare runs FindCommit twice for SHAs it already holds, on every stream and diff_file request), #629873 (fold diffs_stats into the metadata response), #629874 (fail soft on Gitaly errors, plus the product question of anonymous users defaulting to hidden whitespace, which routes all anonymous traffic to the live path). Outside Rapid Diffs, MergeRequest#diff_stats behind GraphQL diffStatsSummary runs DiffStats uncached on every MR widget load.

Closes #629869 Epic: &23646

Edited by Marc Shaw

Merge request reports

Loading
Loading