Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

packfile: create packfile.Index and reuse it #510

Merged
merged 1 commit into from
Jul 26, 2017
Merged

packfile: create packfile.Index and reuse it #510

merged 1 commit into from
Jul 26, 2017

Conversation

smola
Copy link
Collaborator

@smola smola commented Jul 25, 2017

There was an internal type (i.e. storage/filesystem.idx) to
use as in-memory index for packfiles. This was not convenient
to reuse in the packfile.

This commit creates a new representation (format/packfile.Index)
that can be converted to and from idxfile.Idxfile.

A packfile.Index now contains the functionality that was scattered
on storage/filesystem.idx and packfile.Decoder's internals.

storage/filesystem now reuses packfile.Index instances and this
also results in higher cache hit ratios when resolving deltas.

@smola
Copy link
Collaborator Author

smola commented Jul 25, 2017

This is the first PR in a series of 3 to improve performance on various operations, including push. This index structure is required to improve caching and reusing deltas (next 2 PRs).

@mcuadros
Copy link
Contributor

@smola CI

func (d *Decoder) Offsets() map[plumbing.Hash]int64 {
return d.hashToOffset
// SetIndex sets an index for the packfile. It is recommended to set this.
// The index might be read from a file or reused from a previous Decoer usage
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decoer -> Decoder


// FromIdxFile takes an idxfile.Idxfile and adds its entries to the current
// index. The current index will be emptied before adding the new entries.
func (idx *Index) FromIdxFile(idxf *idxfile.Idxfile) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If each Index is only intended to be used on one packfile, Should this method be a constructor?

There was an internal type (i.e. storage/filesystem.idx) to
use as in-memory index for packfiles. This was not convenient
to reuse in the packfile.

This commit creates a new representation (format/packfile.Index)
that can be converted to and from idxfile.Idxfile.

A packfile.Index now contains the functionality that was scattered
on storage/filesystem.idx and packfile.Decoder's internals.

storage/filesystem now reuses packfile.Index instances and this
also results in higher cache hit ratios when resolving deltas.
@codecov
Copy link

codecov bot commented Jul 26, 2017

Codecov Report

Merging #510 into master will decrease coverage by 0.53%.
The diff coverage is 95.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #510      +/-   ##
==========================================
- Coverage   78.29%   77.76%   -0.54%     
==========================================
  Files         127      128       +1     
  Lines        9428     9455      +27     
==========================================
- Hits         7382     7353      -29     
- Misses       1240     1311      +71     
+ Partials      806      791      -15
Impacted Files Coverage Δ
plumbing/format/idxfile/encoder.go 75.38% <ø> (ø) ⬆️
storage/filesystem/internal/dotgit/writers.go 77.03% <100%> (-0.17%) ⬇️
plumbing/format/idxfile/idxfile.go 90.47% <100%> (+1%) ⬆️
plumbing/format/idxfile/decoder.go 62.31% <100%> (ø) ⬆️
plumbing/format/packfile/index.go 100% <100%> (ø)
storage/filesystem/object.go 68.75% <87.5%> (-0.95%) ⬇️
plumbing/format/packfile/decoder.go 81.25% <93.93%> (+2.02%) ⬆️
plumbing/transport/ssh/common.go 2.81% <0%> (-47.89%) ⬇️
plumbing/transport/ssh/auth_method.go 33.33% <0%> (-24.77%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fbf2a4a...c64eb81. Read the comment docs.

@mcuadros mcuadros merged commit d7b898e into src-d:master Jul 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants