Skip to content

Conversation

ajnavarro
Copy link
Contributor

Some refactoring was needed to be able to use the new packfile decoder API from go-git.

Speed performance on full scans and generating indexes of 40%-60%, depending on the dataset.

Signed-off-by: Antonio Jesus Navarro Perez antnavper@gmail.com

packfiles.go Outdated
@@ -193,7 +200,7 @@ func getUnpackedObject(repo repository, hash plumbing.Hash) (o object.Object, er
type repoObjectDecoder struct {
repo string
packfile plumbing.Hash
decoder *packfile.Decoder
decoder *packfile.Packfile
Copy link
Contributor

Choose a reason for hiding this comment

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

should we rename decoder to packfile then?

Copy link
Contributor

Choose a reason for hiding this comment

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

... and remove/rename packfile plumbing.Hash?

@ajnavarro ajnavarro force-pushed the improvement/update-git-version-improved-decoder branch from 5b51805 to 98342bf Compare August 17, 2018 14:48
packfiles.go Outdated
@@ -193,7 +200,7 @@ func getUnpackedObject(repo repository, hash plumbing.Hash) (o object.Object, er
type repoObjectDecoder struct {
repo string
packfile plumbing.Hash
decoder *packfile.Decoder
decoder *packfile.Packfile
Copy link
Contributor

Choose a reason for hiding this comment

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

... and remove/rename packfile plumbing.Hash?

proto.RegisterType((*Bit)(nil), "internal.Bit")
}
if t := proto.MessageType("internal.ColumnAttrSet"); t == nil {
proto.RegisterType((*ColumnAttrSet)(nil), "internal.ColumnAttrSet")
Copy link
Contributor

Choose a reason for hiding this comment

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

good to keep these extra ifs. it avoids printing warning about already registered type, on start.

Copy link
Contributor

Choose a reason for hiding this comment

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

Vendor files should not be modified manually

Copy link
Contributor

@kuba-- kuba-- Aug 17, 2018

Choose a reason for hiding this comment

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

We'll have to find a better way for these autogenerated files, but so far I modified them adding extra ifs to avoid nasty warnings (after integrating pilosalib driver - both try to register the same types in protobuf)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I didn't have protobuf errors on startup anymore. Some dependency must be updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ajnavarro - do you have pilosa running? Right now, if pilosa is not running we don't register old driver. The warnings appeared when we registered both drivers, because pilosa and go-pilosa register in protobuf the same types.

@erizocosmico - I totally agree, but it was just ad-hoc fix to suppress ugly warnings. It was temporary, the quickest fix.

Moreover based on comments in protobufs:
https://github.com/golang/protobuf/blob/master/proto/properties.go#L481
Some day duplicate proto type register may panic.

@ajnavarro
Copy link
Contributor Author

Hmm, I think CI is failing because now the objects are iterated in another order. Can that be the cause @erizocosmico @jfontan ? WDYT?

@erizocosmico
Copy link
Contributor

@ajnavarro it can be that, the errors in travis are not very descriptive, though, so no way to be certain.

@jfontan
Copy link
Contributor

jfontan commented Aug 21, 2018

I'm taking a look on test errors.

@jfontan
Copy link
Contributor

jfontan commented Aug 21, 2018

The problem was the order of the objects returned by the iterator. Sent a PR changing the iterator back to offset order.

src-d/go-git#927

Some refactor was needed to be able to use the new packfile decoder API from go-git.

Speed performance on full scans and generating indexes of 40%-60%, depending of the dataset.

Signed-off-by: Antonio Jesus Navarro Perez <antnavper@gmail.com>
Signed-off-by: Antonio Jesus Navarro Perez <antnavper@gmail.com>
@ajnavarro ajnavarro force-pushed the improvement/update-git-version-improved-decoder branch from 98342bf to de986e5 Compare August 22, 2018 08:04
Signed-off-by: Antonio Jesus Navarro Perez <antnavper@gmail.com>
Signed-off-by: Antonio Jesus Navarro Perez <antnavper@gmail.com>
@ajnavarro ajnavarro merged commit 8fdfe3c into src-d:master Aug 22, 2018
@ajnavarro ajnavarro deleted the improvement/update-git-version-improved-decoder branch August 22, 2018 09:11
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.

5 participants