Skip to content

UPSTREAM PR #2458: fix(tree): std::path::Component has infallible conversion to &[u8]#28

Open
loci-dev wants to merge 2 commits into
mainfrom
loci/pr-2458-direct-u8
Open

UPSTREAM PR #2458: fix(tree): std::path::Component has infallible conversion to &[u8]#28
loci-dev wants to merge 2 commits into
mainfrom
loci/pr-2458-direct-u8

Conversation

@loci-dev
Copy link
Copy Markdown

@loci-dev loci-dev commented Mar 7, 2026

Note

Source pull request: GitoxideLabs/gitoxide#2458

The checks/conversions done in the _by_path methods in Tree and TreeRefIter are unnecessary, so this PR removes them in favor of using OsStr::as_encoded_bytes().

Since as_os_str().as_encoded_bytes() takes &self and (as far as I know) no platforms use internal mutability on OsStr, the underlying value must already have the form we want. The only thing that os_str_into_bstr does for us is validate that the byte string contains valid UTF-8. However, the only thing these BStrs are used for is comparison to other BStrs. Whether either of them is valid UTF-8 does not impact the function in any way (as string conversion errors are silently ignored), so we can simply skip the whole validation step and get ever-so-slightly-cheaper lookups by path.

Please let me know if this change is useful/welcome or not, I can imagine that there may be some case I have not thought of that breaks from this change.

Instead of relying on a fallible path and using a default value,
we can convert directly between a `Component` and a `&[u8]`.
@loci-review
Copy link
Copy Markdown

loci-review Bot commented Mar 7, 2026

Overview

Analysis of 31,034 functions (1,742 modified, 4,771 new, 4,774 removed) across two binaries shows negligible performance impact from path conversion simplification commits.

Binaries analyzed:

  • target.aarch64-unknown-linux-gnu.release.gix: +0.05% power consumption (+401 nJ)
  • target.aarch64-unknown-linux-gnu.release.ein: -0.003% power consumption (-8 nJ)

Commits: Two focused changes simplified std::path::Component to &[u8] conversion (making it infallible) and removed the gix-path dependency from gix-object.

Function Analysis

Most performance variations are compiler-driven code generation differences rather than algorithmic changes from the source modifications:

Performance-critical improvements:

  • gix_diff::rewrites::Tracker::try_push_change: -28% response time (-159ns) — eliminated exception handling overhead in rename detection pipeline

Performance-critical regressions:

  • gix_fs::Stack::make_relative_path_current: +21% response time (+2,619ns) — doubled next_match() calls from 2 to 4, affects status operations and worktree traversal
  • gix_index::extension::Tree::verify_recursive: +54% response time (+1,196ns) — duplicated operations and added memory allocations

Non-critical changes:

  • Standard library thread initialization: +157% response time (+1,199ns)
  • Debug formatting implementations: +43% to +15,659% (only affects error display/logging)
  • Commit graph serialization: +91% response time (+57.8μs, infrequent CLI output)
  • Channel operations: Mixed results (+8% in gix, -4% in ein)
  • Shell completion: -48% response time (-22.9μs improvement)

Other analyzed functions showed negligible changes or compiler optimization variations in external dependencies.

Source code relationship: The path conversion changes in gix-object do not directly affect most regressed functions. Performance variations stem from compiler optimization differences between builds rather than the intentional source modifications, which successfully simplified code without algorithmic regressions.

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

@loci-dev loci-dev force-pushed the main branch 4 times, most recently from 2c4a72b to 167bdd1 Compare March 13, 2026 07:48
@loci-dev loci-dev force-pushed the main branch 3 times, most recently from 9b41e5f to 1f13824 Compare March 21, 2026 07:44
@loci-dev loci-dev force-pushed the main branch 8 times, most recently from 06bc48e to 0e47b1e Compare March 30, 2026 07:05
@loci-dev loci-dev force-pushed the main branch 2 times, most recently from 8b02847 to 1bf0519 Compare April 2, 2026 07:54
@loci-dev loci-dev force-pushed the main branch 3 times, most recently from cdbe120 to 78a7ab5 Compare April 11, 2026 07:49
@loci-dev loci-dev force-pushed the main branch 6 times, most recently from 49231d8 to bc0a777 Compare April 20, 2026 07:18
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.

2 participants