Skip to content

UPSTREAM PR #2408: [WIP] Add test to reproduce commit hiding issue in gix-traverse#16

Open
loci-dev wants to merge 4 commits intomainfrom
loci/pr-2408-copilot-add-test-for-commit-hiding-issue
Open

UPSTREAM PR #2408: [WIP] Add test to reproduce commit hiding issue in gix-traverse#16
loci-dev wants to merge 4 commits intomainfrom
loci/pr-2408-copilot-add-test-for-commit-hiding-issue

Conversation

@loci-dev
Copy link
Copy Markdown

@loci-dev loci-dev commented Feb 3, 2026

Note

Source pull request: GitoxideLabs/gitoxide#2408

Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.

Original prompt

In gix-traverse there is a way to hide commits, and all their ancestry. But the implementation isn't correct as it's not based on "graph painting" via gix-revwalk, and can easily include commits in the traversal because they have not yet been seen by the tips that are hiding them.
Add a test, using gix-testtools, completely ignoring the test-suite and style in gix-traverse, to reproduce this issue, and try to fix it.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@loci-review
Copy link
Copy Markdown

loci-review Bot commented Feb 3, 2026

Overview

Analysis of 26,353 functions across two Gitoxide binaries reveals exceptional stability between versions. Modified functions: 53 (0.20%), new: 152, removed: 152, unchanged: 25,996 (98.64%). Only one function exceeded the 2% performance change threshold.

Power Consumption:

  • target.aarch64-unknown-linux-gnu.release.gix: -0.012% (790,201 nJ vs 790,293 nJ base)
  • target.aarch64-unknown-linux-gnu.release.ein: -0.004% (294,074 nJ vs 294,086 nJ base)

Function Analysis

parse_from (crtstuff.c__ZN12clap_builder6derive6Parser10parse_from in target.aarch64-unknown-linux-gnu.release.ein):

  • Response time: 65,059,668 ns vs 65,069,468 ns base (-9,800 ns, -0.015%)
  • Throughput time: 145.77 ns vs 148.76 ns base (-2.99 ns, -2.01%)

This CLI argument parser from the external clap crate executes once per application startup. The marginal improvements likely stem from compiler optimizations rather than code changes, as commit history shows modifications focused on gix-traverse graph painting implementation, unrelated to CLI parsing. No changes detected in gitoxide's argument structures or parser usage. The function is non-critical—performance-sensitive areas (object database access, pack operations, tree diffing) remain unaffected and unchanged.

🔎 Full breakdown: Loci Inspector.
💬 Questions? Tag @loci-dev.

@loci-dev loci-dev force-pushed the main branch 2 times, most recently from d47eb48 to 10e213a Compare February 3, 2026 10:09
@loci-review
Copy link
Copy Markdown

loci-review Bot commented Feb 3, 2026

Overview

Analysis of 28,525 functions across two Gitoxide binaries reveals targeted performance impacts from a correctness-focused algorithmic change in commit graph traversal. The modification replaces lazy interleaved traversal with eager "graph painting" to ensure topology-independent correctness matching Git's behavior.

Function Counts: 784 modified, 2,324 new, 2,318 removed, 23,099 unchanged

Binaries Analyzed:

  • target.aarch64-unknown-linux-gnu.release.ein: +0.20% power consumption (+575.89 nJ)
  • target.aarch64-unknown-linux-gnu.release.gix: -0.06% power consumption (-500.89 nJ)

Function Analysis

Revision Spec Parsing Functions (ein binary):

Multiple functions in the revision spec parsing pipeline show consistent 3-4x response time increases due to the graph painting algorithm's upfront traversal of hidden commits:

  • handle_errors_and_replacements: 342,132ns → 1,495,066ns (+337%, +1,153μs), throughput stable at ~607ns
  • traverse: 377,900ns → 1,530,720ns (+305%, +1,153μs), throughput +0.15%
  • peel_until: 379,866ns → 1,532,676ns (+303%, +1,153μs), throughput -0.03%
  • done: 388,042ns → 1,540,796ns (+297%, +1,153μs), throughput unchanged

The consistent ~1.15ms increase represents the preprocessing cost of ensuring correct commit filtering through eager graph traversal. All functions maintain stable self-time, confirming the regression originates from the modified gix-traverse infrastructure they depend on.

Commit Iteration Functions:

  • Iterator::nth (both binaries): Throughput time increased 252% (69ns → 244ns), response time +45-252%, reflecting the shift from lazy to eager traversal with additional VecDeque operations and filtering
  • filtered (gix): Throughput +424% (363ns → 1,906ns), response +9%, showing compiler optimization effects from module-level changes
  • Platform::all (ein): Throughput improved -69% (3,123ns → 963ns) but response +11% (256μs → 284μs), demonstrating optimized wrapper logic despite slower child functions

Remote Configuration Functions:

  • config_spec (ein): +549% response time (45μs → 290μs), +212% throughput
  • branch_remote (ein): +147% response time (335μs → 826μs), +0.04% throughput
  • user_agent_tuple (gix): +418% response time (1,025ns → 5,307ns), +0.88% throughput

These functions show no source code changes and have no logical dependency on commit traversal, suggesting compiler optimization artifacts or measurement variance rather than functional regressions.

Standard Library Functions:

VecDeque::spec_extend and Display::fmt show 400-500% throughput increases reflecting altered usage patterns from the graph painting algorithm's increased VecDeque operations and string handling. Other analyzed functions including destructors and error handling utilities saw changes ranging from -76% to +164%, primarily reflecting increased allocation churn from the new algorithm's intermediate data structures.

Additional Findings

The changes demonstrate excellent architectural isolation—impacts are confined to commit traversal operations without affecting performance-critical areas like object access (gix-odb), pack operations, or diff algorithms. The negligible power consumption changes (±0.2%) confirm that localized function regressions don't translate to significant system-wide impacts. All performance costs are justified by correctness improvements that prevent incorrect results in complex repository topologies, embodying the project's correctness-first philosophy. No GPU or ML operations are affected as the project uses CPU-bound algorithms exclusively.

🔎 Full breakdown: Loci Inspector.
💬 Questions? Tag @loci-dev.

@loci-dev loci-dev force-pushed the main branch 3 times, most recently from 4168df6 to ead683a Compare February 3, 2026 13:24
Copilot AI and others added 4 commits February 3, 2026 14:59
…havior

The hide() function now fully traverses all hidden tips and marks all
reachable commits as Hidden before returning. This "graph painting"
approach ensures correct behavior regardless of graph topology or
traversal order, matching git's behavior.

Previously, hidden commits were traversed lazily during iteration,
interleaved with interesting commits. While this worked in most cases,
the new approach is simpler and more robust.

Added tests to verify correct behavior when:
- Hidden tip has longer path to shared ancestor
- Interesting tip has longer path to shared ancestor

Co-authored-by: Byron <[email protected]>
@loci-dev loci-dev force-pushed the loci/pr-2408-copilot-add-test-for-commit-hiding-issue branch from 2e8fccf to 6a5b4e6 Compare February 4, 2026 07:48
@loci-review
Copy link
Copy Markdown

loci-review Bot commented Feb 4, 2026

Overview

Analysis of 32,004 functions across Gitoxide binaries reveals localized performance regressions stemming from a correctness-focused architectural change. Modified functions: 2,370 (7.4%); new: 5,803; removed: 5,800; unchanged: 18,031.

Binaries analyzed:

  • target.aarch64-unknown-linux-gnu.release.ein: +0.229% power consumption (294,086 nJ → 294,759 nJ)
  • target.aarch64-unknown-linux-gnu.release.gix: -0.093% power consumption (790,293 nJ → 789,561 nJ)

Net energy impact is negligible (-0.005%), indicating minimal system-level performance degradation.

Function Analysis

Primary Impact: Revision Spec Parsing (ein binary)

Commit 1617444 implemented "graph painting" in gix-traverse, replacing lazy interleaved traversal with eager upfront BFS traversal to ensure correctness matching Git's rev-list --not semantics. This change propagates through revision parsing:

  • collect_indices_and_mtime_sorted_by_size: Response time +929% (+1.47ms), throughput +0.13%. Regression in called functions; no source changes detected.
  • traverse: Response time +305% (+1.15ms), throughput +0.13%. Inherits graph painting overhead.
  • peel_until: Response time +304% (+1.15ms), throughput -0.01%. Stable self-time confirms regression in callees.
  • handle_errors_and_replacements: Response time +337% (+1.15ms), throughput +0.001%. Called by traverse/peel_until, inherits overhead.
  • done: Response time +297% (+1.15ms), throughput +0.0003%. Finalization affected by upstream changes.

Commit Traversal Impact (gix binary)

  • filtered: Response time +8.6% (+557ns), throughput +429% (+1,560ns). Constructor initialization overhead from graph painting. Performance-critical for filtered commit operations.
  • Iterator::nth (both binaries): Response time +45% (gix), +11% (ein); throughput +252% (+175ns). Shift from lazy to eager evaluation patterns.
  • all: Response time +12% (+31µs), throughput -69% (-2,159ns). Simplified wrapper logic but slower called functions.

Configuration/Remote Operations

  • config_spec (ein): Response time +552% (+247µs), throughput +212% (+237ns). No source changes; likely compiler optimization differences.
  • branch_remote: Response time +148% (+496µs), throughput -0.10%. Unexplained regression in called functions.

Other Functions

  • host_matches: Throughput +121% (+266ns). Pre-existing inefficiency with redundant iterator traversals (3x: two counts + one match).
  • fmt (Display): Throughput +564% (+52ns), response -0.13%. Compiler code generation differences; negligible impact on CLI formatting.
  • drop_in_place (Error): Throughput -76% (-91ns). Compiler optimization improvement.

Remaining analyzed functions showed minimal changes consistent with compiler optimization variations and the bytes crate update (1.11.0→1.11.1).

Additional Findings

The performance regressions are intentional trade-offs prioritizing correctness. Graph painting ensures Gitoxide matches Git's behavior exactly across all graph topologies, addressing potential bugs in complex revision specs. Affected functions are not in performance-critical hot paths identified in project insights (object database, pack operations, diffing). The O(H) initialization cost for H hidden commits is acceptable for correctness guarantees in a Git implementation. No GPU/ML operations were affected.

🔎 Full breakdown: Loci Inspector.
💬 Questions? Tag @loci-dev.

@loci-dev loci-dev force-pushed the main branch 7 times, most recently from a4396e6 to bdb09ad Compare February 11, 2026 07:20
@loci-dev loci-dev force-pushed the main branch 2 times, most recently from 4805387 to 853b34d Compare February 13, 2026 07:49
@loci-dev loci-dev force-pushed the main branch 4 times, most recently from 167bdd1 to 3deba97 Compare March 15, 2026 07:48
@loci-dev loci-dev force-pushed the main branch 5 times, most recently from 0223bcb to bfaee00 Compare March 24, 2026 07:50
@loci-dev loci-dev force-pushed the main branch 6 times, most recently from 0e47b1e to 8b02847 Compare March 31, 2026 07:55
@loci-dev loci-dev force-pushed the main branch 3 times, most recently from 96cdaee to cdbe120 Compare April 9, 2026 07:02
@loci-dev loci-dev force-pushed the main branch 5 times, most recently from 9c2e45d to 5124e99 Compare April 17, 2026 07:07
@loci-dev loci-dev force-pushed the main branch 5 times, most recently from 1c8b114 to e902b63 Compare April 26, 2026 07:07
@loci-dev loci-dev force-pushed the main branch 2 times, most recently from c6739bc to 6f67b12 Compare April 29, 2026 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants