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

storage: reuse deltas from packfiles #515

Merged
merged 3 commits into from
Jul 27, 2017
Merged

storage: reuse deltas from packfiles #515

merged 3 commits into from
Jul 27, 2017

Conversation

smola
Copy link
Collaborator

@smola smola commented Jul 27, 2017

  • plumbing: add DeltaObject interface for EncodedObjects that
    are deltas and hold additional information about them, such
    as the hash of the base object.

  • plumbing/storer: add DeltaObjectStorer interface for object
    storers that can return DeltaObject. Note that calls to
    EncodedObject will never return instances of DeltaObject.
    That requires explicit calls to DeltaObject.

  • storage/filesystem: implement DeltaObjectStorer interface.

  • plumbing/packfile: packfile encoder now supports reusing
    deltas that are already computed (e.g. from an existing
    packfile) if the storage implements DeltaObjectStorer.
    Reusing deltas boosts performance of packfile generation
    (e.g. on push).

return nil
}

func (dw *deltaSelector) fixAndBreakChainsOne(objectsToPack map[plumbing.Hash]*ObjectToPack, otp *ObjectToPack) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

why One ? what does it means?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's one because it processes just one ObjectToPack.

}

do, ok := otp.Object.(plumbing.DeltaObject)
if !ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

1

}

base, ok := objectsToPack[do.BaseHash()]
if !ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

2

return nil
}

if base.Size() <= otp.Size() {
Copy link
Contributor

Choose a reason for hiding this comment

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

3

1,2 and 3 are almost identical pieces of code. Could you find a way to do not duplicate these code? I know that is plumbing code, difficult to improve, but we can try.

return nil
}

isDelta := otp.Object.Type() == plumbing.OFSDeltaObject ||
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a function.

* Improve checks for encode/decode.

* Make it easier to extend this test with more
  storage backends.
@codecov
Copy link

codecov bot commented Jul 27, 2017

Codecov Report

Merging #515 into master will decrease coverage by 0.79%.
The diff coverage is 68.04%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #515     +/-   ##
=========================================
- Coverage   78.28%   77.49%   -0.8%     
=========================================
  Files         128      129      +1     
  Lines        9539     9685    +146     
=========================================
+ Hits         7468     7505     +37     
- Misses       1264     1368    +104     
- Partials      807      812      +5
Impacted Files Coverage Δ
plumbing/object.go 100% <ø> (ø) ⬆️
plumbing/storer/object.go 94.82% <ø> (ø) ⬆️
storage/filesystem/deltaobject.go 50% <50%> (ø)
plumbing/format/packfile/encoder.go 80.85% <60%> (-1.57%) ⬇️
storage/filesystem/object.go 66.52% <64.15%> (-2.39%) ⬇️
plumbing/format/packfile/delta_selector.go 80.15% <72.22%> (-11.03%) ⬇️
plumbing/format/packfile/object_pack.go 87.5% <77.27%> (-12.5%) ⬇️
plumbing/transport/ssh/common.go 2.81% <0%> (-47.89%) ⬇️
plumbing/transport/ssh/auth_method.go 33.33% <0%> (-24.77%) ⬇️
... and 3 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 8c57f96...16b24f8. Read the comment docs.

* plumbing: add DeltaObject interface for EncodedObjects that
  are deltas and hold additional information about them, such
  as the hash of the base object.

* plumbing/storer: add DeltaObjectStorer interface for object
  storers that can return DeltaObject. Note that calls to
  EncodedObject will never return instances of DeltaObject.
  That requires explicit calls to DeltaObject.

* storage/filesystem: implement DeltaObjectStorer interface.

* plumbing/packfile: packfile encoder now supports reusing
  deltas that are already computed (e.g. from an existing
  packfile) if the storage implements DeltaObjectStorer.
  Reusing deltas boosts performance of packfile generation
  (e.g. on push).
ObjectType.IsDelta is a convenience function to match both
offset and reference delta types.
@mcuadros mcuadros merged commit 86f33ed into src-d:master Jul 27, 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.

None yet

3 participants