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

Improve listing performance by using go-git #6478

Merged
merged 39 commits into from
Apr 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
034c4b9
Use go-git for tree reading and commit info lookup.
filipnavara Sep 23, 2018
ed4b558
Use TreeEntry.IsRegular() instead of ObjectType that was removed.
filipnavara Sep 23, 2018
7f21e2f
Use the treePath to optimize commit info search.
filipnavara Sep 23, 2018
268bb79
Extract the latest commit at treePath along with the other commits.
filipnavara Sep 24, 2018
abab5ae
Fix listing commit info for a directory that was created in one commi…
filipnavara Sep 24, 2018
e4f68c6
Avoid nearly all external 'git' invocations when doing directory list…
filipnavara Sep 24, 2018
403596e
Use go-git for reading blobs.
filipnavara Sep 24, 2018
8a5ab23
Make SHA1 type alias for plumbing.Hash in go-git.
filipnavara Sep 24, 2018
77996d6
Make Signature type alias for object.Signature in go-git.
filipnavara Sep 24, 2018
f64b5d5
Fix GetCommitsInfo for repository with only one commit.
filipnavara Sep 24, 2018
97501ea
Fix PGP signature verification.
filipnavara Sep 25, 2018
cced8c1
Fix issues with walking commit graph across merges.
filipnavara Sep 27, 2018
e121188
Fix typo in condition.
filipnavara Oct 1, 2018
9eef362
Speed up loading branch list by keeping the repository reference (and…
filipnavara Oct 18, 2018
a6155d5
Fix lising submodules.
filipnavara Dec 17, 2018
aeb6caf
Fix build
filipnavara Jan 8, 2019
157380e
Add back commit cache because of name-rev
filipnavara Apr 1, 2019
10d74c0
Fix tests
filipnavara Apr 1, 2019
6370c0b
Fix code style
filipnavara Apr 1, 2019
43bfdaf
Fix spelling
filipnavara Apr 1, 2019
35b4419
Address PR feedback
filipnavara Apr 1, 2019
29788d0
Update vendor module list
Apr 3, 2019
e28ede0
Fix getting trees by commit id
filipnavara Apr 5, 2019
63a788e
Fix remaining unit test failures
filipnavara Apr 5, 2019
1a0a1ae
Fix GetTreeBySHA
filipnavara Apr 5, 2019
2801e13
Avoid running `git name-rev` if not necessary
filipnavara Apr 14, 2019
cf4ca84
Move Branch code to git module
filipnavara Apr 14, 2019
c9ff5c5
Clean up GPG signature verification and fix it for tagged commits
filipnavara Apr 14, 2019
8f42467
Merge branch 'master' into perf-read
techknowlogick Apr 17, 2019
b2ab511
Address PR feedback (import formatting, copyright headers)
filipnavara Apr 17, 2019
0e81166
Merge branch 'master' into perf-read
techknowlogick Apr 17, 2019
6b17e9b
Merge remote-tracking branch 'upstream/master' into perf-read
filipnavara Apr 17, 2019
5ddbcd7
Make blob lookup by SHA working
filipnavara Apr 17, 2019
beef9e0
Update tests to use public API
filipnavara Apr 17, 2019
b11cf53
Allow getting content from any type of object through the blob interface
filipnavara Apr 17, 2019
0ee4399
Change test to actually expect the object content that is in the GIT …
filipnavara Apr 17, 2019
e3c3123
Change one more test to actually expect the object content that is in…
filipnavara Apr 17, 2019
ca7e1e6
Merge branch 'master' into perf-read
zeripath Apr 19, 2019
c036283
Add comments
filipnavara Apr 19, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ require (
github.com/dgrijalva/jwt-go v0.0.0-20161101193935-9ed569b5d1ac
github.com/edsrzf/mmap-go v0.0.0-20170320065105-0bce6a688712 // indirect
github.com/elazarl/go-bindata-assetfs v0.0.0-20151224045452-57eb5e1fc594 // indirect
github.com/emirpasic/gods v1.12.0 // indirect
github.com/emirpasic/gods v1.12.0
github.com/etcd-io/bbolt v1.3.2 // indirect
github.com/ethantkoenig/rupture v0.0.0-20180203182544-0a76f03a811a
github.com/facebookgo/clock v0.0.0-20150410010913-600d898af40a // indirect
Expand Down Expand Up @@ -127,7 +127,7 @@ require (
gopkg.in/ldap.v3 v3.0.2
gopkg.in/macaron.v1 v1.3.2
gopkg.in/redis.v2 v2.3.2 // indirect
gopkg.in/src-d/go-billy.v4 v4.3.0 // indirect
gopkg.in/src-d/go-billy.v4 v4.3.0
gopkg.in/src-d/go-git.v4 v4.10.0
gopkg.in/testfixtures.v2 v2.5.0
mvdan.cc/xurls/v2 v2.0.0
Expand Down
2 changes: 1 addition & 1 deletion integrations/api_repo_git_blobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestAPIReposGitBlobs(t *testing.T) {
var gitBlobResponse api.GitBlobResponse
DecodeJSON(t, resp, &gitBlobResponse)
assert.NotNil(t, gitBlobResponse)
expectedContent := "Y29tbWl0IDY1ZjFiZjI3YmMzYmY3MGY2NDY1NzY1ODYzNWU2NjA5NGVkYmNiNGQKQXV0aG9yOiB1c2VyMSA8YWRkcmVzczFAZXhhbXBsZS5jb20+CkRhdGU6ICAgU3VuIE1hciAxOSAxNjo0Nzo1OSAyMDE3IC0wNDAwCgogICAgSW5pdGlhbCBjb21taXQKCmRpZmYgLS1naXQgYS9SRUFETUUubWQgYi9SRUFETUUubWQKbmV3IGZpbGUgbW9kZSAxMDA2NDQKaW5kZXggMDAwMDAwMC4uNGI0ODUxYQotLS0gL2Rldi9udWxsCisrKyBiL1JFQURNRS5tZApAQCAtMCwwICsxLDMgQEAKKyMgcmVwbzEKKworRGVzY3JpcHRpb24gZm9yIHJlcG8xClwgTm8gbmV3bGluZSBhdCBlbmQgb2YgZmlsZQo="
expectedContent := "dHJlZSAyYTJmMWQ0NjcwNzI4YTJlMTAwNDllMzQ1YmQ3YTI3NjQ2OGJlYWI2CmF1dGhvciB1c2VyMSA8YWRkcmVzczFAZXhhbXBsZS5jb20+IDE0ODk5NTY0NzkgLTA0MDAKY29tbWl0dGVyIEV0aGFuIEtvZW5pZyA8ZXRoYW50a29lbmlnQGdtYWlsLmNvbT4gMTQ4OTk1NjQ3OSAtMDQwMAoKSW5pdGlhbCBjb21taXQK"
Copy link
Member

Choose a reason for hiding this comment

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

It's strange before tests PASSed.

Copy link
Contributor Author

@filipnavara filipnavara Apr 19, 2019

Choose a reason for hiding this comment

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

See my comment above. It's actually identical test with identical content.

assert.Equal(t, expectedContent, gitBlobResponse.Content)

// Tests a private repo with no token so will fail
Expand Down
15 changes: 0 additions & 15 deletions models/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -874,21 +874,6 @@ func (err ErrUserDoesNotHaveAccessToRepo) Error() string {
// |______ / |__| (____ /___| /\___ >___| /
// \/ \/ \/ \/ \/

// ErrBranchNotExist represents a "BranchNotExist" kind of error.
type ErrBranchNotExist struct {
Name string
}

// IsErrBranchNotExist checks if an error is a ErrBranchNotExist.
func IsErrBranchNotExist(err error) bool {
_, ok := err.(ErrBranchNotExist)
return ok
}

func (err ErrBranchNotExist) Error() string {
return fmt.Sprintf("branch does not exist [name: %s]", err.Name)
}

// ErrBranchAlreadyExists represents an error that branch with such name already exists.
type ErrBranchAlreadyExists struct {
BranchName string
Expand Down
5 changes: 3 additions & 2 deletions models/pull.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Copyright 2015 The Gogs Authors. All rights reserved.
// Copyright 2019 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

Expand Down Expand Up @@ -165,8 +166,8 @@ func (pr *PullRequest) APIFormat() *api.PullRequest {

filipnavara marked this conversation as resolved.
Show resolved Hide resolved
func (pr *PullRequest) apiFormat(e Engine) *api.PullRequest {
var (
baseBranch *Branch
headBranch *Branch
baseBranch *git.Branch
headBranch *git.Branch
baseCommit *git.Commit
headCommit *git.Commit
err error
Expand Down
57 changes: 10 additions & 47 deletions models/repo_branch.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Copyright 2016 The Gogs Authors. All rights reserved.
// Copyright 2019 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

Expand Down Expand Up @@ -86,53 +87,24 @@ func (repo *Repository) DeleteLocalBranch(branchName string) error {
return deleteLocalBranch(repo.LocalCopyPath(), repo.DefaultBranch, branchName)
filipnavara marked this conversation as resolved.
Show resolved Hide resolved
}

// Branch holds the branch information
type Branch struct {
Path string
Name string
}

// GetBranchesByPath returns a branch by it's path
func GetBranchesByPath(path string) ([]*Branch, error) {
gitRepo, err := git.OpenRepository(path)
if err != nil {
return nil, err
}

brs, err := gitRepo.GetBranches()
if err != nil {
return nil, err
}

branches := make([]*Branch, len(brs))
for i := range brs {
branches[i] = &Branch{
Path: path,
Name: brs[i],
}
}
return branches, nil
}

// CanCreateBranch returns true if repository meets the requirements for creating new branches.
func (repo *Repository) CanCreateBranch() bool {
return !repo.IsMirror
}

// GetBranch returns a branch by it's name
func (repo *Repository) GetBranch(branch string) (*Branch, error) {
if !git.IsBranchExist(repo.RepoPath(), branch) {
return nil, ErrBranchNotExist{branch}
// GetBranch returns a branch by its name
func (repo *Repository) GetBranch(branch string) (*git.Branch, error) {
gitRepo, err := git.OpenRepository(repo.RepoPath())
if err != nil {
return nil, err
}
return &Branch{
Path: repo.RepoPath(),
Name: branch,
}, nil

return gitRepo.GetBranch(branch)
}

// GetBranches returns all the branches of a repository
func (repo *Repository) GetBranches() ([]*Branch, error) {
return GetBranchesByPath(repo.RepoPath())
func (repo *Repository) GetBranches() ([]*git.Branch, error) {
return git.GetBranchesByPath(repo.RepoPath())
}

// CheckBranchName validates branch name with existing repository branches
Expand Down Expand Up @@ -257,12 +229,3 @@ func (repo *Repository) CreateNewBranchFromCommit(doer *User, commit, branchName

return nil
}

// GetCommit returns all the commits of a branch
func (branch *Branch) GetCommit() (*git.Commit, error) {
gitRepo, err := git.OpenRepository(branch.Path)
if err != nil {
return nil, err
}
return gitRepo.GetBranchCommit(branch.Name)
}
3 changes: 2 additions & 1 deletion modules/context/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,11 @@ func (r *Repository) GetEditorconfig() (*editorconfig.Editorconfig, error) {
if treeEntry.Blob().Size() >= setting.UI.MaxDisplayFileSize {
return nil, git.ErrNotExist{ID: "", RelPath: ".editorconfig"}
}
reader, err := treeEntry.Blob().Data()
reader, err := treeEntry.Blob().DataAsync()
if err != nil {
return nil, err
}
defer reader.Close()
data, err := ioutil.ReadAll(reader)
if err != nil {
return nil, err
Expand Down
66 changes: 15 additions & 51 deletions modules/git/blob.go
Original file line number Diff line number Diff line change
@@ -1,76 +1,40 @@
// Copyright 2015 The Gogs Authors. All rights reserved.
// Copyright 2019 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package git
filipnavara marked this conversation as resolved.
Show resolved Hide resolved

import (
"bytes"
"encoding/base64"
"fmt"
"io"
"io/ioutil"
"os"
"os/exec"

"gopkg.in/src-d/go-git.v4/plumbing"
)

// Blob represents a Git object.
type Blob struct {
repo *Repository
*TreeEntry
}

// Data gets content of blob all at once and wrap it as io.Reader.
// This can be very slow and memory consuming for huge content.
func (b *Blob) Data() (io.Reader, error) {
stdout := new(bytes.Buffer)
stderr := new(bytes.Buffer)

// Preallocate memory to save ~50% memory usage on big files.
stdout.Grow(int(b.Size() + 2048))

if err := b.DataPipeline(stdout, stderr); err != nil {
return nil, concatenateError(err, stderr.String())
}
return stdout, nil
}
ID SHA1

// DataPipeline gets content of blob and write the result or error to stdout or stderr
func (b *Blob) DataPipeline(stdout, stderr io.Writer) error {
return NewCommand("show", b.ID.String()).RunInDirPipeline(b.repo.Path, stdout, stderr)
}

type cmdReadCloser struct {
cmd *exec.Cmd
stdout io.Reader
}

func (c cmdReadCloser) Read(p []byte) (int, error) {
return c.stdout.Read(p)
}

func (c cmdReadCloser) Close() error {
io.Copy(ioutil.Discard, c.stdout)
return c.cmd.Wait()
gogitEncodedObj plumbing.EncodedObject
name string
}

// DataAsync gets a ReadCloser for the contents of a blob without reading it all.
// Calling the Close function on the result will discard all unread output.
func (b *Blob) DataAsync() (io.ReadCloser, error) {
cmd := exec.Command("git", "show", b.ID.String())
cmd.Dir = b.repo.Path
cmd.Stderr = os.Stderr

stdout, err := cmd.StdoutPipe()
if err != nil {
return nil, fmt.Errorf("StdoutPipe: %v", err)
}
return b.gogitEncodedObj.Reader()
}

if err = cmd.Start(); err != nil {
return nil, fmt.Errorf("Start: %v", err)
}
// Size returns the uncompressed size of the blob
func (b *Blob) Size() int64 {
return b.gogitEncodedObj.Size()
}

return cmdReadCloser{stdout: stdout, cmd: cmd}, nil
// Name returns name of the tree entry this blob object was created from (or empty string)
func (b *Blob) Name() string {
return b.name
}

// GetBlobContentBase64 Reads the content of the blob with a base64 encode and returns the encoded string
Expand Down
45 changes: 18 additions & 27 deletions modules/git/blob_test.go
Original file line number Diff line number Diff line change
@@ -1,32 +1,18 @@
// Copyright 2015 The Gogs Authors. All rights reserved.
// Copyright 2019 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package git
filipnavara marked this conversation as resolved.
Show resolved Hide resolved

import (
"bytes"
"io/ioutil"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

var repoSelf = &Repository{
Path: "./",
}

var testBlob = &Blob{
repo: repoSelf,
TreeEntry: &TreeEntry{
ID: MustIDFromString("a8d4b49dd073a4a38a7e58385eeff7cc52568697"),
ptree: &Tree{
repo: repoSelf,
},
},
}

func TestBlob_Data(t *testing.T) {
output := `Copyright (c) 2016 The Gitea Authors
Copyright (c) 2015 The Gogs Authors
Expand All @@ -49,32 +35,37 @@ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.
`
repo, err := OpenRepository("../../.git")
assert.NoError(t, err)
testBlob, err := repo.GetBlob("a8d4b49dd073a4a38a7e58385eeff7cc52568697")
assert.NoError(t, err)

r, err := testBlob.Data()
r, err := testBlob.DataAsync()
assert.NoError(t, err)
require.NotNil(t, r)
defer r.Close()

data, err := ioutil.ReadAll(r)
assert.NoError(t, err)
assert.Equal(t, output, string(data))
}

func Benchmark_Blob_Data(b *testing.B) {
for i := 0; i < b.N; i++ {
r, err := testBlob.Data()
if err != nil {
b.Fatal(err)
}
ioutil.ReadAll(r)
repo, err := OpenRepository("../../.git")
if err != nil {
b.Fatal(err)
}
testBlob, err := repo.GetBlob("a8d4b49dd073a4a38a7e58385eeff7cc52568697")
if err != nil {
b.Fatal(err)
}
}

func Benchmark_Blob_DataPipeline(b *testing.B) {
stdout := new(bytes.Buffer)
for i := 0; i < b.N; i++ {
stdout.Reset()
if err := testBlob.DataPipeline(stdout, nil); err != nil {
r, err := testBlob.DataAsync()
if err != nil {
b.Fatal(err)
}
defer r.Close()
ioutil.ReadAll(r)
}
}
Loading