Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove need for Polonius borrow checker in git_odb::compound::Db::locate(…) #66

Closed
Byron opened this issue Apr 7, 2021 · 1 comment

Comments

@Byron
Copy link
Member

Byron commented Apr 7, 2021

Currently there is branching code in git_odb::compound::Db::locate(…) to workaround a borrowcheck shortcoming using the upcoming Polonius borrow checker.

https://github.com/Byron/gitoxide/blob/b317200b72096573d511d229c6e61e74e7ba14db/git-odb/src/compound/locate.rs#L32-L42

Without the workaround, the lookup costs are doubled.

Using Polonius solves the problem but comes at a cost.

  • It requires the nightly compiler (forcing gitoxide to be compiled with nightly if that code-path is chosen).
  • The borrow checker performance is reduced and can 'explode' for certain crates like tinyvec.
  • It's unclear when Polonius will be stable.

Here is @joshtriplett thoughts on how to resolve this:

When first initializing the Db and looking up all the alternates, rather than recursing, collect all of the alternates as a top-level Vec of (Packs, Loose), so that alternates themselves never have alternates. That way, you don't have to recurse, and there's only one level of alternates. Then, in the loop over alternates, rather than recursively calling locate, just inline the same code that looks in packs and loose, but without the check for alternates.
I think the simplest way to do that would be to have a version of at (at_no_alternates) that ignores alternates entirely, and then have the top-level at collect a deduplicated list of alternates and call at_no_alternates on each one.

Additional Information

Also have a look at how this is currently done for looking up objects in packs. The gist is to get an index of where the object is found first, bypassing the need for borrowing an output buffer, and fill the buffer if an index was found.

https://github.com/Byron/gitoxide/blob/b317200b72096573d511d229c6e61e74e7ba14db/git-odb/src/compound/locate.rs#L28-L30

@Byron Byron mentioned this issue Apr 10, 2021
43 tasks
@Byron
Copy link
Member Author

Byron commented Apr 13, 2021

  • linked repo type with init
  • linked repo locate()
  • support for caches in locate()
  • object-access experiment uses new repo
  • docs

@Byron Byron closed this as completed Apr 13, 2021
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

No branches or pull requests

1 participant