From 99d45c0e8ea8b33f08ff4481a5b61ca254a80477 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Wed, 9 Jul 2014 07:59:55 -0400 Subject: [PATCH] godoc/analysis: show analysis status in UI (source file view) Also: - declare PackageInfo, FileInfo types to simplify API. - update docs: eliminate this TODO item document improved analysis times state that -analysis=pointer implies -analysis=type LGTM=gri R=gri CC=golang-codereviews https://golang.org/cl/112770044 --- godoc/analysis/README | 7 --- godoc/analysis/analysis.go | 90 ++++++++++++++++++++++----------- godoc/server.go | 28 +++++----- godoc/static/analysis/help.html | 13 ++--- godoc/static/static.go | 13 ++--- 5 files changed, 82 insertions(+), 69 deletions(-) diff --git a/godoc/analysis/README b/godoc/analysis/README index 6913b9796d..411af1cbaf 100644 --- a/godoc/analysis/README +++ b/godoc/analysis/README @@ -59,7 +59,6 @@ Type info: - Suppress toggle divs for empty method sets. Misc: -- Add an "analysis help" page explaining the features and UI in more detail. - The [X] button in the lower pane is subject to scrolling. - Should the lower pane be floating? An iframe? When we change document.location by clicking on a link, it will go away. @@ -110,9 +109,3 @@ contains a lot of filenames (e.g. 820 copies of 16 distinct filenames). 20% of the HTML is L%d spans (now disabled). The HTML also contains lots of tooltips for long struct/interface types. De-dup or just abbreviate? The actual formatting is very fast. - -The pointer analysis constraint solver is way too slow. We really -need to implement the constraint optimizer. Also: performance has -been quite unpredictable; I recently optimized it fourfold with -type-based label tracking, but the benefit seems to have evaporated. -TODO(adonovan): take a look at recent CLs for regressions. diff --git a/godoc/analysis/analysis.go b/godoc/analysis/analysis.go index 9fabe118ca..7bf4dc0fe3 100644 --- a/godoc/analysis/analysis.go +++ b/godoc/analysis/analysis.go @@ -123,6 +123,13 @@ func (e errorLink) Write(w io.Writer, _ int, start bool) { // -- fileInfo --------------------------------------------------------- +// FileInfo holds analysis information for the source file view. +// Clients must not mutate it. +type FileInfo struct { + Data []interface{} // JSON serializable values + Links []Link // HTML link markup +} + // A fileInfo is the server's store of hyperlinks and JSON data for a // particular file. type fileInfo struct { @@ -154,20 +161,28 @@ func (fi *fileInfo) addData(x interface{}) int { return index } -// get returns new slices containing opaque JSON values and the HTML link markup for fi. -// Callers must not mutate the elements. -func (fi *fileInfo) get() (data []interface{}, links []Link) { +// get returns the file info in external form. +// Callers must not mutate its fields. +func (fi *fileInfo) get() FileInfo { + var r FileInfo // Copy slices, to avoid races. fi.mu.Lock() - data = append(data, fi.data...) + r.Data = append(r.Data, fi.data...) if !fi.sorted { sort.Sort(linksByStart(fi.links)) fi.sorted = true } - links = append(links, fi.links...) + r.Links = append(r.Links, fi.links...) fi.mu.Unlock() + return r +} - return +// PackageInfo holds analysis information for the package view. +// Clients must not mutate it. +type PackageInfo struct { + CallGraph []*PCGNodeJSON + CallGraphIndex map[string]int + Types []*TypeInfoJSON } type pkgInfo struct { @@ -190,16 +205,17 @@ func (pi *pkgInfo) addType(t *TypeInfoJSON) { pi.mu.Unlock() } -// get returns new slices of JSON values for the callgraph and type info for pi. -// Callers must not mutate the slice elements or the map. -func (pi *pkgInfo) get() (callGraph []*PCGNodeJSON, callGraphIndex map[string]int, types []*TypeInfoJSON) { +// get returns the package info in external form. +// Callers must not mutate its fields. +func (pi *pkgInfo) get() PackageInfo { + var r PackageInfo // Copy slices, to avoid races. pi.mu.Lock() - callGraph = append(callGraph, pi.callGraph...) - callGraphIndex = pi.callGraphIndex - types = append(types, pi.types...) + r.CallGraph = append(r.CallGraph, pi.callGraph...) + r.CallGraphIndex = pi.callGraphIndex + r.Types = append(r.Types, pi.types...) pi.mu.Unlock() - return + return r } // -- Result ----------------------------------------------------------- @@ -209,6 +225,7 @@ func (pi *pkgInfo) get() (callGraph []*PCGNodeJSON, callGraphIndex map[string]in // and JavaScript data referenced by the links. type Result struct { mu sync.Mutex // guards maps (but not their contents) + status string // global analysis status fileInfos map[string]*fileInfo // keys are godoc file URLs pkgInfos map[string]*pkgInfo // keys are import paths } @@ -229,12 +246,26 @@ func (res *Result) fileInfo(url string) *fileInfo { return fi } +// Status returns a human-readable description of the current analysis status. +func (res *Result) Status() string { + res.mu.Lock() + defer res.mu.Unlock() + return res.status +} + +func (res *Result) setStatusf(format string, args ...interface{}) { + res.mu.Lock() + res.status = fmt.Sprintf(format, args...) + log.Printf(format, args...) + res.mu.Unlock() +} + // FileInfo returns new slices containing opaque JSON values and the // HTML link markup for the specified godoc file URL. Thread-safe. // Callers must not mutate the elements. // It returns "zero" if no data is available. // -func (res *Result) FileInfo(url string) ([]interface{}, []Link) { +func (res *Result) FileInfo(url string) (fi FileInfo) { return res.fileInfo(url).get() } @@ -256,10 +287,10 @@ func (res *Result) pkgInfo(importPath string) *pkgInfo { // PackageInfo returns new slices of JSON values for the callgraph and // type info for the specified package. Thread-safe. -// Callers must not mutate the elements. +// Callers must not mutate its fields. // PackageInfo returns "zero" if no data is available. // -func (res *Result) PackageInfo(importPath string) ([]*PCGNodeJSON, map[string]int, []*TypeInfoJSON) { +func (res *Result) PackageInfo(importPath string) PackageInfo { return res.pkgInfo(importPath).get() } @@ -333,17 +364,18 @@ func Run(pta bool, result *Result) { //args = []string{"fmt"} if _, err := conf.FromArgs(args, true); err != nil { - log.Printf("Analysis failed: %s\n", err) // import error + // TODO(adonovan): degrade gracefully, not fail totally. + result.setStatusf("Analysis failed: %s.", err) // import error return } - log.Print("Loading and type-checking packages...") + result.setStatusf("Loading and type-checking packages...") iprog, err := conf.Load() if iprog != nil { log.Printf("Loaded %d packages.", len(iprog.AllPackages)) } if err != nil { - log.Printf("Analysis failed: %s\n", err) + result.setStatusf("Loading failed: %s.\n", err) return } @@ -365,9 +397,9 @@ func Run(pta bool, result *Result) { log.Print("Main packages: ", mainPkgs) // Build SSA code for bodies of all functions in the whole program. - log.Print("Building SSA...") + result.setStatusf("Constructing SSA form...") prog.BuildAll() - log.Print("SSA building complete") + log.Print("SSA construction complete") a := analysis{ result: result, @@ -443,19 +475,18 @@ func Run(pta bool, result *Result) { } } } - log.Print("Computing implements...") + log.Print("Computing implements relation...") facts := computeImplements(&a.prog.MethodSets, a.allNamed) // Add the type-based analysis results. log.Print("Extracting type info...") - for _, info := range iprog.AllPackages { a.doTypeInfo(info, facts) } a.visitInstrs(pta) - log.Print("Extracting type info complete") + result.setStatusf("Type analysis complete.") if pta { a.pointer(mainPkgs) @@ -514,23 +545,24 @@ func (a *analysis) pointer(mainPkgs []*ssa.Package) { a.ptaConfig.BuildCallGraph = true a.ptaConfig.Reflection = false // (for now) - log.Print("Running pointer analysis...") + a.result.setStatusf("Pointer analysis running...") + ptares, err := pointer.Analyze(&a.ptaConfig) if err != nil { // If this happens, it indicates a bug. - log.Printf("Pointer analysis failed: %s", err) + a.result.setStatusf("Pointer analysis failed: %s.", err) return } log.Print("Pointer analysis complete.") // Add the results of pointer analysis. - log.Print("Computing channel peers...") + a.result.setStatusf("Computing channel peers...") a.doChannelPeers(ptares.Queries) - log.Print("Computing dynamic call graph edges...") + a.result.setStatusf("Computing dynamic call graph edges...") a.doCallgraph(ptares.CallGraph) - log.Print("Done") + a.result.setStatusf("Analysis complete.") } type linksByStart []Link diff --git a/godoc/server.go b/godoc/server.go index 76fc48b45d..f81a02d778 100644 --- a/godoc/server.go +++ b/godoc/server.go @@ -260,14 +260,13 @@ func (h *handlerServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { } // Emit JSON array for type information. - // TODO(adonovan): issue a "pending..." message if results not ready. - var callGraph []*analysis.PCGNodeJSON - var typeInfos []*analysis.TypeInfoJSON - callGraph, info.CallGraphIndex, typeInfos = h.c.Analysis.PackageInfo(relpath) - info.CallGraph = htmltemplate.JS(marshalJSON(callGraph)) - info.AnalysisData = htmltemplate.JS(marshalJSON(typeInfos)) + // TODO(adonovan): display the h.c.Analysis.Status() message in the UI. + pi := h.c.Analysis.PackageInfo(relpath) + info.CallGraphIndex = pi.CallGraphIndex + info.CallGraph = htmltemplate.JS(marshalJSON(pi.CallGraph)) + info.AnalysisData = htmltemplate.JS(marshalJSON(pi.Types)) info.TypeInfoIndex = make(map[string]int) - for i, ti := range typeInfos { + for i, ti := range pi.Types { info.TypeInfoIndex[ti.Name] = i } @@ -501,20 +500,19 @@ func (p *Presentation) serveTextFile(w http.ResponseWriter, r *http.Request, abs var buf bytes.Buffer if pathpkg.Ext(abspath) == ".go" { // Find markup links for this file (e.g. "/src/pkg/fmt/print.go"). - data, links := p.Corpus.Analysis.FileInfo(abspath) + fi := p.Corpus.Analysis.FileInfo(abspath) buf.WriteString("\n") - // TODO(adonovan): indicate whether analysis is - // disabled, pending, completed or failed. - // For now, display help link only if 'completed'. - if links != nil { - buf.WriteString("Static analysis features
") + if status := p.Corpus.Analysis.Status(); status != "" { + buf.WriteString("Static analysis features ") + // TODO(adonovan): show analysis status at per-file granularity. + fmt.Fprintf(&buf, "[%s]
", htmlpkg.EscapeString(status)) } buf.WriteString("
")
-		formatGoSource(&buf, src, links, h, s)
+		formatGoSource(&buf, src, fi.Links, h, s)
 		buf.WriteString("
") } else { buf.WriteString("
")
diff --git a/godoc/static/analysis/help.html b/godoc/static/analysis/help.html
index 30ed4a6ebd..61f0665145 100644
--- a/godoc/static/analysis/help.html
+++ b/godoc/static/analysis/help.html
@@ -39,7 +39,7 @@
   expression and the method set of each type, and determines which
   types are assignable to each interface type.
 
-  Type analysis is relatively quick, requiring just a few seconds for
+  Type analysis is relatively quick, requiring about 10 seconds for
   the >200 packages of the standard library, for example.
 

 
@@ -101,7 +101,7 @@
 
 
Pointer analysis features

 

-  godoc -analysis=pointer performs a precise
+  godoc -analysis=pointer additionally performs a precise
   whole-program pointer analysis.  In other words, it
   approximates the set of memory locations to which each
   reference—not just vars of kind *T, but also
@@ -113,10 +113,8 @@
   channel.
 

 

-  ⚠ Pointer analysis is currently quite slow,
-  taking around two minutes for the standard library.  This will
-  improve markedly with the planned addition of a constraint
-  optimizer.
+  Pointer analysis is slower than type analysis, taking an additional
+  15 seconds or so for the standard libraries and their tests.
 

 
 
Call graph navigation

@@ -250,9 +248,6 @@
 
 
Known issues

 

-  ⚠ There is no UI indication of the state of
-  the analysis (pending, complete, failed) during warm-up.

-
   ⚠ All analysis results pertain to exactly
   one configuration (e.g. amd64 linux).  Files that are conditionally
   compiled based on different platforms or build tags are not visible
diff --git a/godoc/static/static.go b/godoc/static/static.go
index 6b288833bc..625ac16ea4 100644
--- a/godoc/static/static.go
+++ b/godoc/static/static.go
@@ -60,7 +60,7 @@ var Files = map[string]string{
   expression and the method set of each type, and determines which
   types are assignable to each interface type.
 
-  Type analysis is relatively quick, requiring just a few seconds for
+  Type analysis is relatively quick, requiring about 10 seconds for
   the >200 packages of the standard library, for example.
 

 
@@ -122,7 +122,7 @@ var Files = map[string]string{
 
 
Pointer analysis features

 

-  godoc -analysis=pointer performs a precise
+  godoc -analysis=pointer additionally performs a precise
   whole-program pointer analysis.  In other words, it
   approximates the set of memory locations to which each
   reference—not just vars of kind *T, but also
@@ -134,10 +134,8 @@ var Files = map[string]string{
   channel.
 

 

-  ⚠ Pointer analysis is currently quite slow,
-  taking around two minutes for the standard library.  This will
-  improve markedly with the planned addition of a constraint
-  optimizer.
+  Pointer analysis is slower than type analysis, taking an additional
+  15 seconds or so for the standard libraries and their tests.
 

 
 
Call graph navigation

@@ -271,9 +269,6 @@ var Files = map[string]string{
 
 
Known issues

 

-  ⚠ There is no UI indication of the state of
-  the analysis (pending, complete, failed) during warm-up.

-
   ⚠ All analysis results pertain to exactly
   one configuration (e.g. amd64 linux).  Files that are conditionally
   compiled based on different platforms or build tags are not visible