From 51bb3c308c459b5c2604c8dd27cc8d4c378c7f43 Mon Sep 17 00:00:00 2001 From: cdeverau Date: Wed, 13 May 2026 11:40:52 -0700 Subject: [PATCH 1/2] Convert _parse_diff to callback-based iteration Replace the indexed git_patch_get_line_in_hunk loop in _parse_diff with libgit2's git_diff_foreach callback API. The previous implementation walked libgit2's internal line list for every line accessor call, producing O(n^2) cost on hunks with many lines (e.g. large modified .tscn files). The callback path is O(n) and is libgit2's intended streaming consumer pattern, mirroring what _get_line_diff already does via diff_hunk_cb. First step toward fixing #211. A byte-budget cap and pre-patch file-size short-circuit are planned as follow-ups; the remaining freeze is dominated by the engine-side RichTextLabel renderer. --- godot-git-plugin/src/git_callbacks.cpp | 69 ++++++++++++++++++++++++++ godot-git-plugin/src/git_callbacks.h | 23 +++++++++ godot-git-plugin/src/git_plugin.cpp | 43 +++------------- 3 files changed, 100 insertions(+), 35 deletions(-) diff --git a/godot-git-plugin/src/git_callbacks.cpp b/godot-git-plugin/src/git_callbacks.cpp index b655e1cb..ac59c894 100644 --- a/godot-git-plugin/src/git_callbacks.cpp +++ b/godot-git-plugin/src/git_callbacks.cpp @@ -107,3 +107,72 @@ extern "C" int diff_hunk_cb(const git_diff_delta *delta, const git_diff_hunk *ra return 1; } + +void ParseDiffPayload::flush_hunk() { + if (!has_current_hunk) { + return; + } + current_hunk = git_plugin->add_line_diffs_into_diff_hunk(current_hunk, current_hunk_lines); + current_file_hunks.push_back(current_hunk); + has_current_hunk = false; + current_hunk = godot::Dictionary(); + current_hunk_lines = godot::TypedArray(); +} + +void ParseDiffPayload::flush_file() { + flush_hunk(); + if (!has_current_file) { + return; + } + current_file = git_plugin->add_diff_hunks_into_diff_file(current_file, current_file_hunks); + diff_contents->push_back(current_file); + has_current_file = false; + current_file = godot::Dictionary(); + current_file_hunks = godot::TypedArray(); +} + +extern "C" int parse_diff_file_cb(const git_diff_delta *delta, float progress, void *payload) { + (void)progress; + ParseDiffPayload *state = (ParseDiffPayload *)payload; + state->flush_file(); + state->current_file = state->git_plugin->create_diff_file( + godot::String::utf8(delta->new_file.path), + godot::String::utf8(delta->old_file.path)); + state->has_current_file = true; + return 0; +} + +extern "C" int parse_diff_binary_cb(const git_diff_delta *delta, const git_diff_binary *binary, void *payload) { + (void)delta; + (void)binary; + (void)payload; + return 0; +} + +extern "C" int parse_diff_hunk_cb(const git_diff_delta *delta, const git_diff_hunk *hunk, void *payload) { + (void)delta; + ParseDiffPayload *state = (ParseDiffPayload *)payload; + state->flush_hunk(); + state->current_hunk = state->git_plugin->create_diff_hunk( + hunk->old_start, hunk->new_start, hunk->old_lines, hunk->new_lines); + state->has_current_hunk = true; + return 0; +} + +extern "C" int parse_diff_line_cb(const git_diff_delta *delta, const git_diff_hunk *hunk, const git_diff_line *line, void *payload) { + (void)delta; + (void)hunk; + ParseDiffPayload *state = (ParseDiffPayload *)payload; + + char *content = new char[line->content_len + 1]; + std::memcpy(content, line->content, line->content_len); + content[line->content_len] = '\0'; + + godot::String status = " "; + status[0] = line->origin; + state->current_hunk_lines.push_back(state->git_plugin->create_diff_line( + line->new_lineno, line->old_lineno, godot::String::utf8(content), status)); + + delete[] content; + return 0; +} diff --git a/godot-git-plugin/src/git_callbacks.h b/godot-git-plugin/src/git_callbacks.h index 57015a94..05d552c2 100644 --- a/godot-git-plugin/src/git_callbacks.h +++ b/godot-git-plugin/src/git_callbacks.h @@ -1,6 +1,8 @@ #pragma once #include "godot_cpp/variant/array.hpp" +#include "godot_cpp/variant/dictionary.hpp" +#include "godot_cpp/variant/typed_array.hpp" #include "git2.h" class GitPlugin; @@ -10,6 +12,22 @@ struct DiffHelper { GitPlugin *git_plugin; }; +struct ParseDiffPayload { + GitPlugin *git_plugin = nullptr; + godot::TypedArray *diff_contents = nullptr; + + bool has_current_file = false; + godot::Dictionary current_file; + godot::TypedArray current_file_hunks; + + bool has_current_hunk = false; + godot::Dictionary current_hunk; + godot::TypedArray current_hunk_lines; + + void flush_hunk(); + void flush_file(); +}; + extern "C" int progress_cb(const char *str, int len, void *data); extern "C" int update_cb(const char *refname, const git_oid *a, const git_oid *b, void *data); extern "C" int transfer_progress_cb(const git_indexer_progress *stats, void *payload); @@ -18,3 +36,8 @@ extern "C" int credentials_cb(git_cred **out, const char *url, const char *usern extern "C" int push_transfer_progress_cb(unsigned int current, unsigned int total, size_t bytes, void *payload); extern "C" int push_update_reference_cb(const char *refname, const char *status, void *data); extern "C" int diff_hunk_cb(const git_diff_delta *delta, const git_diff_hunk *range, void *payload); + +extern "C" int parse_diff_file_cb(const git_diff_delta *delta, float progress, void *payload); +extern "C" int parse_diff_binary_cb(const git_diff_delta *delta, const git_diff_binary *binary, void *payload); +extern "C" int parse_diff_hunk_cb(const git_diff_delta *delta, const git_diff_hunk *hunk, void *payload); +extern "C" int parse_diff_line_cb(const git_diff_delta *delta, const git_diff_hunk *hunk, const git_diff_line *line, void *payload); diff --git a/godot-git-plugin/src/git_plugin.cpp b/godot-git-plugin/src/git_plugin.cpp index af7360a1..055c2cd7 100644 --- a/godot-git-plugin/src/git_plugin.cpp +++ b/godot-git-plugin/src/git_plugin.cpp @@ -624,44 +624,17 @@ godot::TypedArray GitPlugin::_get_diff(const godot::String &i godot::TypedArray GitPlugin::_parse_diff(git_diff *diff) { godot::TypedArray diff_contents; - for (int i = 0; i < git_diff_num_deltas(diff); i++) { - const git_diff_delta *delta = git_diff_get_delta(diff, i); - git_patch_ptr patch; - GIT2_CALL_R(git_patch_from_diff(Capture(patch), diff, i), "Could not create patch from diff", godot::TypedArray()); + ParseDiffPayload state; + state.git_plugin = this; + state.diff_contents = &diff_contents; - godot::Dictionary diff_file = create_diff_file(godot::String::utf8(delta->new_file.path), godot::String::utf8(delta->old_file.path)); + GIT2_CALL_R( + git_diff_foreach(diff, parse_diff_file_cb, parse_diff_binary_cb, parse_diff_hunk_cb, parse_diff_line_cb, &state), + "Could not iterate diff", + godot::TypedArray()); - godot::TypedArray diff_hunks; - for (int j = 0; j < git_patch_num_hunks(patch.get()); j++) { - const git_diff_hunk *git_hunk; - size_t line_count; - GIT2_CALL_R(git_patch_get_hunk(&git_hunk, &line_count, patch.get(), j), "Could not get hunk from patch", godot::TypedArray()); - - godot::Dictionary diff_hunk = create_diff_hunk(git_hunk->old_start, git_hunk->new_start, git_hunk->old_lines, git_hunk->new_lines); - - godot::TypedArray diff_lines; - for (int k = 0; k < line_count; k++) { - const git_diff_line *git_diff_line; - GIT2_CALL_R(git_patch_get_line_in_hunk(&git_diff_line, patch.get(), j, k), "Could not get line from hunk in patch", godot::TypedArray()); - - char *content = new char[git_diff_line->content_len + 1]; - std::memcpy(content, git_diff_line->content, git_diff_line->content_len); - content[git_diff_line->content_len] = '\0'; - - godot::String status = " "; // We reserve 1 null terminated space to fill the + or the - character at git_diff_line->origin - status[0] = git_diff_line->origin; - diff_lines.push_back(create_diff_line(git_diff_line->new_lineno, git_diff_line->old_lineno, godot::String::utf8(content), status)); - - delete[] content; - } - - diff_hunk = add_line_diffs_into_diff_hunk(diff_hunk, diff_lines); - diff_hunks.push_back(diff_hunk); - } - diff_file = add_diff_hunks_into_diff_file(diff_file, diff_hunks); - diff_contents.push_back(diff_file); - } + state.flush_file(); return diff_contents; } From 411b2e9475a63902e0cb0ffef677494ccbcf2fd4 Mon Sep 17 00:00:00 2001 From: cdeverau Date: Wed, 13 May 2026 12:02:33 -0700 Subject: [PATCH 2/2] Cap per-file diff output at 256 KB / 5000 lines Add a truncation state machine to ParseDiffPayload that short-circuits line marshalling once a per-file budget is reached. line_cb checks the projected byte count and line count before allocating, so a single multi-megabyte line (issue #211's worst case) trips the cap on its first invocation with zero allocations. Once truncated, subsequent hunk_cb / line_cb calls for the same file return immediately, and a synthetic context line is emitted as a sentinel so the user sees an explicit "diff truncated, use an external git client" annotation instead of silent omission. Counters reset per file in flush_file. Cuts staging-panel clicks on large inline-resource diffs from a multi-second freeze to effectively instant; the residual cost is engine-side RichTextLabel rendering of the lines that fit under the cap. --- godot-git-plugin/src/git_callbacks.cpp | 39 ++++++++++++++++++++++++++ godot-git-plugin/src/git_callbacks.h | 12 ++++++++ 2 files changed, 51 insertions(+) diff --git a/godot-git-plugin/src/git_callbacks.cpp b/godot-git-plugin/src/git_callbacks.cpp index ac59c894..51a45840 100644 --- a/godot-git-plugin/src/git_callbacks.cpp +++ b/godot-git-plugin/src/git_callbacks.cpp @@ -1,4 +1,5 @@ #include +#include #include #include "git_callbacks.h" @@ -129,6 +130,19 @@ void ParseDiffPayload::flush_file() { has_current_file = false; current_file = godot::Dictionary(); current_file_hunks = godot::TypedArray(); + current_file_bytes = 0; + current_file_lines = 0; + current_file_truncated = false; +} + +void ParseDiffPayload::begin_synthetic_truncation_hunk(const char *message) { + flush_hunk(); + current_hunk = git_plugin->create_diff_hunk(0, 0, 0, 0); + has_current_hunk = true; + godot::String status = " "; + current_hunk_lines.push_back(git_plugin->create_diff_line( + 0, 0, godot::String::utf8(message), status)); + current_file_truncated = true; } extern "C" int parse_diff_file_cb(const git_diff_delta *delta, float progress, void *payload) { @@ -152,6 +166,9 @@ extern "C" int parse_diff_binary_cb(const git_diff_delta *delta, const git_diff_ extern "C" int parse_diff_hunk_cb(const git_diff_delta *delta, const git_diff_hunk *hunk, void *payload) { (void)delta; ParseDiffPayload *state = (ParseDiffPayload *)payload; + if (state->current_file_truncated) { + return 0; + } state->flush_hunk(); state->current_hunk = state->git_plugin->create_diff_hunk( hunk->old_start, hunk->new_start, hunk->old_lines, hunk->new_lines); @@ -163,6 +180,28 @@ extern "C" int parse_diff_line_cb(const git_diff_delta *delta, const git_diff_hu (void)delta; (void)hunk; ParseDiffPayload *state = (ParseDiffPayload *)payload; + if (state->current_file_truncated) { + return 0; + } + + if (state->limits.byte_budget > 0 && state->current_file_bytes + line->content_len > state->limits.byte_budget) { + char msg[256]; + std::snprintf(msg, sizeof(msg), + "... diff truncated: %zu KB limit reached. Use an external git client to view the full diff. ...\n", + state->limits.byte_budget / 1024); + state->begin_synthetic_truncation_hunk(msg); + return 0; + } + if (state->limits.line_budget > 0 && state->current_file_lines >= state->limits.line_budget) { + char msg[256]; + std::snprintf(msg, sizeof(msg), + "... diff truncated: %zu-line limit reached. Use an external git client to view the full diff. ...\n", + state->limits.line_budget); + state->begin_synthetic_truncation_hunk(msg); + return 0; + } + state->current_file_bytes += line->content_len; + state->current_file_lines += 1; char *content = new char[line->content_len + 1]; std::memcpy(content, line->content, line->content_len); diff --git a/godot-git-plugin/src/git_callbacks.h b/godot-git-plugin/src/git_callbacks.h index 05d552c2..ace2e906 100644 --- a/godot-git-plugin/src/git_callbacks.h +++ b/godot-git-plugin/src/git_callbacks.h @@ -12,10 +12,17 @@ struct DiffHelper { GitPlugin *git_plugin; }; +struct ParseDiffLimits { + size_t byte_budget = 256 * 1024; // Per-file content cap. + size_t line_budget = 5000; // Per-file line cap. +}; + struct ParseDiffPayload { GitPlugin *git_plugin = nullptr; godot::TypedArray *diff_contents = nullptr; + ParseDiffLimits limits; + bool has_current_file = false; godot::Dictionary current_file; godot::TypedArray current_file_hunks; @@ -24,8 +31,13 @@ struct ParseDiffPayload { godot::Dictionary current_hunk; godot::TypedArray current_hunk_lines; + size_t current_file_bytes = 0; + size_t current_file_lines = 0; + bool current_file_truncated = false; + void flush_hunk(); void flush_file(); + void begin_synthetic_truncation_hunk(const char *message); }; extern "C" int progress_cb(const char *str, int len, void *data);