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.

Sign up for GitHub

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

Jump to bottom

Refactor Find traits #267

Merged
merged 51 commits into from
Dec 5, 2021
Merged

Refactor Find traits #267

merged 51 commits into from
Dec 5, 2021

Conversation

Byron
Copy link
Member

@Byron Byron commented Nov 30, 2021 •
edited
Loading

Untangle and simplify the Find trait

Tasks

  • add contains(oid) to trait
  • review the need for bundle::Location and if it's worth the effort. If so, see if it can be taken out of the Object type, which would allow it to move to git-object.
    • split into simple Find trait and packed::Find trait, the latter providing more information for speedups
    • assure pack-ids are static by assigning usize ids while remembering their hash, assuring the small ids don't ever conflict while assigning the same id to each pack (in design sketch), to simplify it.
    • remove pack-cache argument from packed::Find as well for consistency and accepting that handle-based implementations have advantages. This also means that a pack-cache must be leveraged internally and probably is trait-based to allow it to be a no-op.
    • Implement Find for the linked/static db
    • Figure out how to deal with this low-level bundle access in git_pack::Find - probably better to split it up so it's compatible with multi-pack indices
    • Add a 'handle' implementation for odb and use it everywhere
      • Make sure trait implementations are 'static, too, if they have to be, which means no internal OwnShared :/, must probably be generic over a Deref implementation like the linked iterator.
      • use in pack-create
      • Make object-cache an implementation detail for pack::Find, and make pack/create.rs enable it.
      • use it in git-repository state and make it configurable like before, maybe adapting the handle API even
      • It's clear that there only needs to be one Easy, thread-safe, and maybe an easy-local for the Rc based version. Keep the 'Access' abstraction though, because if this goes, State is Easy, and everything is compile time. Actually remove it, cargo smart-release shows the API in action and it seems like there is an unnecessary layer. The reason Repository stays is merely for having something that is Sync.
    • use the Find trait where possible.
    • consider making 'single-threadedness' opt-in, basically inverting the meaning of parallel. That way, it's the 'right' choice for most applications today, and can be opted in by some with other requirements.
      • doesn't work, cargo features are additive, and so are optional dependencies.
  • write docs

@Byron Byron force-pushed the find-refactor branch 2 times, most recently from 3bd9261 to 87930a1 Compare December 2, 2021 02:21
Byron added 20 commits December 3, 2021 08:03
@Byron
Add 'contains()' method to Find (#266)
dfdd6fb 
The plan is to separate pack location entirely from Object and put
the location specific functions into a separate trait.
@Byron
feat: add Data object (#266)
a0bb652 
This is typed data baked by a slice for conversion into parsed ObjectRef's
for example.

This is usually the result of a `Find` operation on an object database.
@Byron
Fix build
7162ed4
@Byron
feat: A simplified version of the Find trait (#266)
e25f4ea 
It's meant for the next generation of object db handles which keep a
local cache of all the details of the actual object database.
@Byron
refactor!: move git_pack::data::Object to git_object::Data, massively…
2290d00 
… alter git_odb::Find trait (#266)

This will break a lot, but has to happen to prepare these traits for the
next generation of object databases.
@Byron
fix docs (#266)
1bb4253
@Byron
feat: linked::Store sorts bundles by modification date, newest first (#…
a81b333 
…266)
@Byron
fix: linked::Store now assures unique IDs across compound stores (#260)
b605c1f
@Byron
Clarify that we really need stable pack ids (#266)
cefc0fa
@Byron
refactor!: remove pack-cache from Find::try_find(…) (#266)
ebc7f47 
With the new architecture this can be an implementation detail without
forcing it to be Sync.
@Byron
Inform when multi-threaded counting is rejected (#266)
cc3b070
@Byron
Use Deref instead of Borrow in linked ODB iterator (#266)
a96b1e5 
Borrow is the manual form, Deref has allows for more automatic use
and more idiomatic looking code.
@Byron
feat: add linked::Store::rc_iter() (#266)
36fde72 
For completeness in case of single-threaded operations
@Byron
git-odb::Find implementation for linked::Store (#266)
9235106
@Byron
refactor (#266)
3310d8f
@Byron
refactor (#266)
1361c31
@Byron
A first tiny stab at Handle for linked store (#266)
ef08f7f 
It could easily be general over all kinds of store as long as there
is support for pack-caches, which might be helpful later for the 'final'
store type.

Ideally, this one now shows how to do it.
@Byron
refactor (#266)
ab14f70
@Byron
construct a handle from a linked store (#266)
9702ed4
@Byron
feat: git_pack::Find::try_find_cached(…, pack_cache) (#266)
58c2edb 
With this method it's easier to bypass local caches and control
the cache oneself entirely.
Byron added 7 commits December 3, 2021 08:31
@Byron
MultiPackIndex compatible pack::Find trait definition (#266)
5fa1a9d 
We avoid using Bundle and boil it down to what it really needs to be
to build packs.
Now it should be mostly ready.
@Byron
odb::Find for Arc and Rc (#266)
b1c82a7
@Byron
git_pack::Find implementation for Rc (#266)
f48630b
@Byron
Make odb handle more general (#266)
2be6725
@Byron
make all handle caches and cache creators optional (#266)
3c30769 
This saves virtual method calls and allocations, and probably
is cleaner too as one doesn't have to know that the default is
a no-op cache.
@Byron
FAIL: Try to make Handle usable for pack creation (#266)
424c9b3 
It's nearly there, but for some reason the boxed dyn traits don't get to
be Send even though it's specified.
@Byron
Cache-creators are indeed shared across threads, must be sync (#266)
c326cb3
Byron added 24 commits December 3, 2021 10:58
@Byron
change!: remove make_object_cache parameter from `git_pack::data::o…
3f05fea 
…utput::count::objects()` (#266)

It now is an implementation detail of the Find trait.
@Byron
Adjust pack-create to changes in git-pack (#266)
12db899
@Byron
Also use object cache if there is no pack-cache (#266)
3e1ae25
@Byron
change!: remove Repository::refresh_object_database() (#266)
93db4a5 
With the linked DB this is simply not possible anymore and we expect
these updates to happen automatically in future for greater convenience.

For now, in order to refresh a repository, one has to reopen it.
@Byron
Add odb handle to state (#266)
4e38da3
@Byron
change!: Rename Repository::odb to Repository::objects (#266)
57de915 
This way it's more inline with `Repository::refs`.
@Byron
Adapt to changes in git-repository (#266)
3266c47
@Byron
change!: fully rely on OdbHandle in repository State (#266)
5e7aa16
@Byron
Adapt to changes in git-repository (#266)
3ab9b03
@Byron
impl Write for Arc, Rc and shared borrows (#266)
5cdc27d
@Byron
Use db handle for writing (#266)
053e7b6
@Byron
Remove unnecessary error variants now that repo() is called less (#266)
afcd579
@Byron
change!: rename easy::State to easy::Handle (#266)
83d7b31 
As the first step to remove the 'Easy' abstraction.
@Byron
refactor!: remove Easy… abstraction in favor of Handle (#266)
b2cc0c6 
This great reduction of complexity allows for being multi-threading
capabie by default with the option to turn that off at compile time.

All `to|into_easy…()` methods are removed in favor of `to_easy()`
along with the removal of all `Easy` types in favor of the single
`easy::Handle`.
@Byron
Adjustments to match changes in git-repository (#266)
117d5f8
@Byron
refactor!: remove borrowing Repo as possible failure cause (#266)
7a91212 
The `easy::Handle` is now a full (but shared) clone of the original
Rpeository with additional thread-local state, hence there is no more
need for a way to access the original repository.
@Byron
Adjustments due to change in git-repository (#266)
e44dc4d
@Byron
fix git-repository docs (#266)
3496a97
@Byron
change!: use git_odb::Find* traits in prelude, instead of `git_pack…
f9c0493 
…::Find*` (#266)

These are higher-level and generally more desirable.
The Find traits in `git-pack` are more useful internally when packs
have to be handled directly, for example when generating packs.
@Byron
adapt to changes in git-repository (#266)
fae309b
@Byron
make fmt
066f3ff
@Byron
make use of a pack cache when fetching commits (#266)
0a11764
@Byron
Improve diffing example to compare performance, right now it looks li…
3a3ad2e 
…ke a regression (#266)

Could be due to the new implementation going through a refcell, whereas
the previous one didn't.

That's fair, I guess, it gets quite a few calls, but also means that
counting with tree-diff got about 10% slower now that the cache is
built-in, maybe.

For max-speed, one would probably have a local cache to avoid
unnecessary object lookups for previously obtained information
alltogether.
@Byron
Use handle with default cache configuration in 'ein hours' (#266)
f71806f 
It's a tiny bit slower, with or without cache, when going through
the handle.

It's interesting as it appears going through an Arc is measurable
on that level already.
@Byron Byron mentioned this pull request Dec 5, 2021
Closed
5 tasks
@Byron Byron merged commit f71806f into main Dec 5, 2021
@Byron Byron deleted the find-refactor branch January 10, 2022 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@Byron