Skip to content

Commit

Permalink
Merge pull request #28 from snipem/develop-memory-consumption
Browse files Browse the repository at this point in the history
Fix excessive memory consumption
  • Loading branch information
snipem authored Apr 12, 2020
2 parents d9505f4 + 87b487a commit 99ace64
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 34 deletions.
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,8 @@ monako
/compose
test/configs/testgenerated/config.testgenerated.yaml
test/configs/testgenerated/menu.testgenerated.md

# Heap analysis

origin*heap*log
*.log
2 changes: 1 addition & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"program": "${workspaceFolder}/cmd/monako/main.go",
"args": [
"-test.run",
"TestMain"
"TestMainMonakoTest"
]
},
{
Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ run_local: build
./monako -config test/config.local.yaml -menu-config test/config.menu.local.md
$(MAKE) serve

trace:
go test -trace=tmp/trace.out ./cmd/monako
go tool trace ./cmd/monako/ tmp/trace.out

compose:
./monako \
-fail-on-error \
Expand Down
2 changes: 1 addition & 1 deletion cmd/monako/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"golang.org/x/net/html"
)

func TestMain(t *testing.T) {
func TestMainMonakoTest(t *testing.T) {

targetDir := GetLocalTempDir(t)
err := os.Chdir(targetDir)
Expand Down
28 changes: 26 additions & 2 deletions pkg/compose/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,32 @@ func (config *Config) Compose() {
if config.Origins[i].FileBlacklist == nil {
config.Origins[i].FileBlacklist = config.FileBlacklist
}
config.Origins[i].CloneDir()
config.Origins[i].ComposeDir()

filesystem := config.Origins[i].CloneDir()
config.Origins[i].ComposeDir(filesystem)

// After processing the origin, delete repo for freeing up memory
// containing the whole virtual filesystem. Can easily add up to
// multiple gigabyte
config.Origins[i].repo = nil

// Performance analysis ------

// Frees up some more megabyte
// debug.FreeOSMemory()

// if os.Getenv("MONAKO_LOG_HEAP") == "true" {

// f, err := os.Create(filepath.Join(fmt.Sprintf("origin_%d.heap.fix.log", i)))
// if err != nil {
// log.Fatal(err)
// }
// pprof.WriteHeapProfile(f)
// f.Close()
// }

// End Performance analysis ------

}

}
Expand Down
8 changes: 4 additions & 4 deletions pkg/compose/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,13 @@ func BenchmarkWholeRepoHugoRepositoryWholeRepo(b *testing.B) {
}

origin.FileWhitelist = origin.config.FileWhitelist
origin.CloneDir()
filesystem := origin.CloneDir()

// Don't fetch commit info early
b.Run("Get Commit Info for Hugo", func(b *testing.B) {

for n := 0; n < b.N; n++ {
origin.ComposeDir()
origin.ComposeDir(filesystem)
}

})
Expand Down Expand Up @@ -164,11 +164,11 @@ func BenchmarkWholeRepoSlowRepository(b *testing.B) {

origin.FileWhitelist = origin.config.FileWhitelist

origin.CloneDir()
filesystem := origin.CloneDir()
b.Run("Get Commit Info", func(b *testing.B) {

for n := 0; n < b.N; n++ {
origin.ComposeDir()
origin.ComposeDir(filesystem)
}

})
Expand Down
47 changes: 33 additions & 14 deletions pkg/compose/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import (

"github.com/snipem/monako/internal/workarounds"
"github.com/snipem/monako/pkg/helpers"
"gopkg.in/src-d/go-billy.v4"
"gopkg.in/src-d/go-git.v4"
"gopkg.in/src-d/go-git.v4/plumbing/object"
"gopkg.in/yaml.v2"
)

Expand All @@ -29,7 +29,7 @@ const standardFilemode = os.FileMode(0700)
type OriginFile struct {

// Commit is the commit info about this file
Commit *object.Commit
Commit *OriginFileCommit
// RemotePath is the path in the origin repository
RemotePath string
// LocalPath is the absolute path on the local disk
Expand All @@ -39,16 +39,29 @@ type OriginFile struct {
parentOrigin *Origin
}

func (file *OriginFile) composeFile() {
// OriginFileCommit represents a commit
type OriginFileCommit struct {
Hash string
Author OriginFileCommitter
Date time.Time
}

// OriginFileCommitter represents the committer of a commit
type OriginFileCommitter struct {
Name string
Email string
}

func (file *OriginFile) composeFile(filesystem billy.Filesystem) {

file.createParentDir()
contentFormat := file.GetFormat()

switch contentFormat {
case Asciidoc, Markdown:
file.copyMarkupFile()
file.copyMarkupFile(filesystem)
default:
file.copyRegularFile()
file.copyRegularFile(filesystem)
}
fmt.Printf("%s -> %s\n", file.RemotePath, file.LocalPath)

Expand All @@ -75,10 +88,9 @@ func (file *OriginFile) createParentDir() {
}
}

func (file *OriginFile) copyRegularFile() {
func (file *OriginFile) copyRegularFile(filesystem billy.Filesystem) {

origin := file.parentOrigin
f, err := origin.filesystem.Open(file.RemotePath)
f, err := filesystem.Open(file.RemotePath)

if err != nil {
log.Fatal(err)
Expand All @@ -96,7 +108,7 @@ func (file *OriginFile) copyRegularFile() {

// getCommitInfo returns the Commit Info for a given file of the repository
// identified by it's filename
func getCommitInfo(remotePath string, repo *git.Repository) (*object.Commit, error) {
func getCommitInfo(remotePath string, repo *git.Repository) (*OriginFileCommit, error) {

log.Debugf("Getting commit info for %s", remotePath)

Expand Down Expand Up @@ -125,14 +137,21 @@ func getCommitInfo(remotePath string, repo *git.Repository) (*object.Commit, err
// This has to be here, otherwise the iterator will return garbage
defer cIter.Close()

return returnCommit, nil
return &OriginFileCommit{
Author: OriginFileCommitter{
Name: returnCommit.Author.Name,
Email: returnCommit.Author.Email,
},
Date: returnCommit.Author.When,
Hash: returnCommit.Hash.String(),
}, nil
}

func (file *OriginFile) copyMarkupFile() {
func (file *OriginFile) copyMarkupFile(filesystem billy.Filesystem) {

// TODO: Only use strings not []byte

bf, err := file.parentOrigin.filesystem.Open(file.RemotePath)
bf, err := filesystem.Open(file.RemotePath)
if err != nil {
log.Fatalf("Error copying markup file %s", err)
}
Expand Down Expand Up @@ -199,11 +218,11 @@ MonakoGitLastCommitAuthorEmail: %s
file.Commit.Hash,
getWebLinkForGitCommit(
file.parentOrigin.URL,
file.Commit.Hash.String(),
file.Commit.Hash,
),
// Use lastMod because other variables won't be parsed as date by Hugo
// Resulting in no date format functions on the file
file.Commit.Author.When.Format(time.RFC3339),
file.Commit.Date.Format(time.RFC3339),
file.Commit.Author.Name,
file.Commit.Author.Email)

Expand Down
24 changes: 13 additions & 11 deletions pkg/compose/origin.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ const Markdown = "MARKDOWN"

// CloneDir clones a HTTPS or lokal Git repository with the given branch and optional username and password.
// A virtual filesystem is returned containing the cloned files.
func (origin *Origin) CloneDir() {
func (origin *Origin) CloneDir() (filesystem billy.Filesystem) {

fmt.Printf("\nCloning in to '%s' with branch '%s' ...\n", origin.URL, origin.Branch)
log.Debugf("Start cloning of %s", origin.URL)

origin.filesystem = memfs.New()
filesystem = memfs.New()

basicauth := http.BasicAuth{}

Expand All @@ -48,7 +48,7 @@ func (origin *Origin) CloneDir() {
}

// TODO Check if we can check out less depth. Like depth = 1
repo, err := git.Clone(memory.NewStorage(), origin.filesystem, &git.CloneOptions{
repo, err := git.Clone(memory.NewStorage(), filesystem, &git.CloneOptions{
URL: origin.URL,
Depth: 0, // problem with depth = 1 is that git log from older commits, can't be accessed
ReferenceName: plumbing.ReferenceName(fmt.Sprintf("refs/heads/%s", origin.Branch)),
Expand All @@ -63,6 +63,8 @@ func (origin *Origin) CloneDir() {
origin.repo = repo
log.Debugf("Ended cloning of %s", origin.URL)

return filesystem

}

// Origin contains all information for a document origin
Expand All @@ -78,23 +80,22 @@ type Origin struct {

Files []OriginFile

repo *git.Repository
config *Config
filesystem billy.Filesystem
repo *git.Repository
config *Config
}

// ComposeDir copies a subdir of a virtual filesystem to a target in the local relative filesystem.
// The copied files can be limited by a whitelist. The Git repository is used to obtain Git commit
// information
func (origin *Origin) ComposeDir() {
origin.Files = origin.getMatchingFiles(origin.SourceDir)
func (origin *Origin) ComposeDir(filesystem billy.Filesystem) {
origin.Files = origin.getMatchingFiles(origin.SourceDir, filesystem)

if len(origin.Files) == 0 {
log.Printf("Found no matching files in '%s' with branch '%s' in folder '%s'\n", origin.URL, origin.Branch, origin.SourceDir)
}

for _, file := range origin.Files {
file.composeFile()
file.composeFile(filesystem)
}
}

Expand All @@ -108,11 +109,11 @@ func NewOrigin(url string, branch string, sourceDir string, targetDir string) *O
return o
}

func (origin *Origin) getMatchingFiles(startdir string) []OriginFile {
func (origin *Origin) getMatchingFiles(startdir string, filesystem billy.Filesystem) []OriginFile {

var originFiles []OriginFile

files, _ := origin.filesystem.ReadDir(startdir)
files, _ := filesystem.ReadDir(startdir)
for _, file := range files {

// This is the path as stored in the remote repo
Expand All @@ -127,6 +128,7 @@ func (origin *Origin) getMatchingFiles(startdir string) []OriginFile {
originFiles,
origin.getMatchingFiles(
remotePath,
filesystem,
)...)
} else if helpers.FileIsListed(file.Name(), origin.FileWhitelist) &&
!helpers.FileIsListed(file.Name(), origin.FileBlacklist) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/compose/origin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func TestGitCommiter(t *testing.T) {
t.Run("Retrieve commit info of file", func(t *testing.T) {
if files.IsContentFile(firstOrigin.Files[i].RemotePath) {
ci := firstOrigin.Files[i].Commit
assert.Contains(t, ci.Committer.Email, "@")
assert.Contains(t, ci.Author.Email, "@")
} else {
t.Logf("Skipping commit check for %s, is not a content file", firstOrigin.Files[i].RemotePath)
}
Expand Down

0 comments on commit 99ace64

Please sign in to comment.