From 76e18544676ff5c7ffbb264c8e0dedf0a2d325da Mon Sep 17 00:00:00 2001 From: Jordan Krage Date: Mon, 21 Aug 2017 06:35:12 -0500 Subject: [PATCH 1/3] verbose logging: add counts to vendor directory lists --- internal/gps/identifier.go | 4 +++ internal/gps/result.go | 57 ++++++++++++++++++++++++++------------ txn_writer.go | 5 ++-- 3 files changed, 46 insertions(+), 20 deletions(-) diff --git a/internal/gps/identifier.go b/internal/gps/identifier.go index de24e88c34..c9ea195275 100644 --- a/internal/gps/identifier.go +++ b/internal/gps/identifier.go @@ -148,6 +148,10 @@ func (i ProjectIdentifier) errString() string { return fmt.Sprintf("%s (from %s)", i.ProjectRoot, i.Source) } +func (i ProjectIdentifier) String() string { + return i.errString() +} + func (i ProjectIdentifier) normalize() ProjectIdentifier { if i.Source == "" { i.Source = string(i.ProjectRoot) diff --git a/internal/gps/result.go b/internal/gps/result.go index 16e4f05268..379f365447 100644 --- a/internal/gps/result.go +++ b/internal/gps/result.go @@ -9,7 +9,6 @@ import ( "log" "os" "path/filepath" - "sync" "github.com/pkg/errors" ) @@ -65,37 +64,59 @@ func WriteDepTree(basedir string, l Lock, sm SourceManager, sv bool, logger *log return err } - var wg sync.WaitGroup - errCh := make(chan error, len(l.Projects())) + lps := l.Projects() + + type resp struct { + i int + err error + } + respCh := make(chan resp, len(lps)) + + for i := range lps { + go func(i int) { + p := lps[i] + + var err error + defer func() { + if r := recover(); r != nil { + err = errors.Errorf("recovered from panic exporting %s: %s", p.Ident().ProjectRoot, r) + } + respCh <- resp{i, err} + }() - for _, p := range l.Projects() { - wg.Add(1) - go func(p LockedProject) { - defer wg.Done() to := filepath.FromSlash(filepath.Join(basedir, string(p.Ident().ProjectRoot))) - logger.Printf("Writing out %s@%s", p.Ident().errString(), p.Version()) - if err := sm.ExportProject(p.Ident(), p.Version(), to); err != nil { - errCh <- errors.Wrapf(err, "failed to export %s", p.Ident().ProjectRoot) + if err = sm.ExportProject(p.Ident(), p.Version(), to); err != nil { + err = errors.Wrapf(err, "failed to export %s", p.Ident().ProjectRoot) return } if sv { - err := filepath.Walk(to, stripVendor) + err = filepath.Walk(to, stripVendor) if err != nil { - errCh <- errors.Wrapf(err, "failed to strip vendor from %s", p.Ident().ProjectRoot) + err = errors.Wrapf(err, "failed to strip vendor from %s", p.Ident().ProjectRoot) } } - }(p) + }(i) } - wg.Wait() - close(errCh) + var errs []error + for i := 0; i < len(lps); i++ { + resp := <-respCh + msg := "Wrote" + if resp.err != nil { + errs = append(errs, resp.err) + msg = "Failed to write" + } + p := lps[resp.i] + logger.Printf("(%d/%d) %s %s@%s\n", i+1, len(lps), msg, p.Ident(), p.Version()) + } + close(respCh) - if len(errCh) > 0 { + if len(errs) > 0 { logger.Println("Failed to write dep tree. The following errors occurred:") - for err := range errCh { - logger.Println(" * ", err) + for i, err := range errs { + logger.Printf("(%d/%d) %s\n", i+1, len(errs), err) } removeAll(basedir) diff --git a/txn_writer.go b/txn_writer.go index 224e3eb52b..adb3c473dd 100644 --- a/txn_writer.go +++ b/txn_writer.go @@ -455,8 +455,9 @@ func (sw *SafeWriter) PrintPreparedActions(output *log.Logger, verbose bool) err if sw.writeVendor { if verbose { output.Printf("Would have written the following %d projects to the vendor directory:\n", len(sw.lock.Projects())) - for _, project := range sw.lock.Projects() { - output.Println(project) + lps := sw.lock.Projects() + for i, p := range lps { + output.Printf("(%d/%d) %s@%s\n", i+1, len(lps), p.Ident(), p.Version()) } } else { output.Printf("Would have written %d projects to the vendor directory.\n", len(sw.lock.Projects())) From ef47c48c37fb54f81c973791eee2f982041eaa63 Mon Sep 17 00:00:00 2001 From: Jordan Krage Date: Sun, 27 Aug 2017 09:50:31 -0500 Subject: [PATCH 2/3] limit # routines writing dep tree; abort on error --- internal/gps/result.go | 80 ++++++++++++++++++++++++++++++------------ 1 file changed, 57 insertions(+), 23 deletions(-) diff --git a/internal/gps/result.go b/internal/gps/result.go index 379f365447..3392e2f6a4 100644 --- a/internal/gps/result.go +++ b/internal/gps/result.go @@ -9,6 +9,8 @@ import ( "log" "os" "path/filepath" + "runtime" + "sync" "github.com/pkg/errors" ) @@ -71,47 +73,79 @@ func WriteDepTree(basedir string, l Lock, sm SourceManager, sv bool, logger *log err error } respCh := make(chan resp, len(lps)) + writeCh := make(chan int, len(lps)) + cancel := make(chan struct{}) + // Queue work. for i := range lps { - go func(i int) { - p := lps[i] - - var err error - defer func() { - if r := recover(); r != nil { - err = errors.Errorf("recovered from panic exporting %s: %s", p.Ident().ProjectRoot, r) + writeCh <- i + } + close(writeCh) + // Launch writers. + writers := runtime.GOMAXPROCS(-1) + if len(lps) < writers { + writers = len(lps) + } + var wg sync.WaitGroup + wg.Add(writers) + for i := 0; i < writers; i++ { + go func() { + defer wg.Done() + + for i := range writeCh { + select { + case <-cancel: + return + default: } - respCh <- resp{i, err} - }() - to := filepath.FromSlash(filepath.Join(basedir, string(p.Ident().ProjectRoot))) + p := lps[i] + to := filepath.FromSlash(filepath.Join(basedir, string(p.Ident().ProjectRoot))) - if err = sm.ExportProject(p.Ident(), p.Version(), to); err != nil { - err = errors.Wrapf(err, "failed to export %s", p.Ident().ProjectRoot) - return - } + if err := sm.ExportProject(p.Ident(), p.Version(), to); err != nil { + respCh <- resp{i, errors.Wrapf(err, "failed to export %s", p.Ident().ProjectRoot)} + continue + } - if sv { - err = filepath.Walk(to, stripVendor) - if err != nil { - err = errors.Wrapf(err, "failed to strip vendor from %s", p.Ident().ProjectRoot) + if sv { + select { + case <-cancel: + return + default: + } + + if err := filepath.Walk(to, stripVendor); err != nil { + respCh <- resp{i, errors.Wrapf(err, "failed to strip vendor from %s", p.Ident().ProjectRoot)} + continue + } } + + respCh <- resp{i, nil} } - }(i) + }() } + // Monitor writers + go func() { + wg.Wait() + close(respCh) + }() + // Log results and collect errors var errs []error - for i := 0; i < len(lps); i++ { - resp := <-respCh + var cnt int + for resp := range respCh { + cnt++ msg := "Wrote" if resp.err != nil { + if len(errs) == 0 { + close(cancel) + } errs = append(errs, resp.err) msg = "Failed to write" } p := lps[resp.i] - logger.Printf("(%d/%d) %s %s@%s\n", i+1, len(lps), msg, p.Ident(), p.Version()) + logger.Printf("(%d/%d) %s %s@%s\n", cnt, len(lps), msg, p.Ident(), p.Version()) } - close(respCh) if len(errs) > 0 { logger.Println("Failed to write dep tree. The following errors occurred:") From 1b50999befe8da57952aca5a13ef1bd513195d7a Mon Sep 17 00:00:00 2001 From: Jordan Krage Date: Tue, 29 Aug 2017 06:11:00 -0500 Subject: [PATCH 3/3] gps: change WriteDepTree concurrent writers from GOMAXPROCS to 16 --- internal/gps/result.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/gps/result.go b/internal/gps/result.go index 3392e2f6a4..c8818f8f82 100644 --- a/internal/gps/result.go +++ b/internal/gps/result.go @@ -9,7 +9,6 @@ import ( "log" "os" "path/filepath" - "runtime" "sync" "github.com/pkg/errors" @@ -47,6 +46,8 @@ type solution struct { solv Solver } +const concurrentWriters = 16 + // WriteDepTree takes a basedir and a Lock, and exports all the projects // listed in the lock to the appropriate target location within the basedir. // @@ -82,7 +83,7 @@ func WriteDepTree(basedir string, l Lock, sm SourceManager, sv bool, logger *log } close(writeCh) // Launch writers. - writers := runtime.GOMAXPROCS(-1) + writers := concurrentWriters if len(lps) < writers { writers = len(lps) }