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

@@ -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.

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.
+// get returns the file info in external form.
-// Callers must not mutate the elements.
+// Callers must not mutate its fields.
-func (fi *fileInfo) get() (data []interface{}, links []Link) {
+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.
+// get returns the package info in external form.
-// Callers must not mutate the slice elements or the map.
+// Callers must not mutate its fields.
-func (pi *pkgInfo) get() (callGraph []*PCGNodeJSON, callGraphIndex map[string]int, types []*TypeInfoJSON) {
+func (pi *pkgInfo) get() PackageInfo {
+	var r PackageInfo
 	// Copy slices, to avoid races.
 	pi.mu.Lock()
-	callGraph = append(callGraph, pi.callGraph...)
+	r.CallGraph = append(r.CallGraph, pi.callGraph...)
-	callGraphIndex = pi.callGraphIndex
+	r.CallGraphIndex = pi.callGraphIndex
-	types = append(types, pi.types...)
+	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

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.
+	// TODO(adonovan): display the h.c.Analysis.Status() message in the UI.
-	var callGraph []*analysis.PCGNodeJSON
+	pi := h.c.Analysis.PackageInfo(relpath)
-	var typeInfos []*analysis.TypeInfoJSON
+	info.CallGraphIndex = pi.CallGraphIndex
-	callGraph, info.CallGraphIndex, typeInfos = h.c.Analysis.PackageInfo(relpath)
+	info.CallGraph = htmltemplate.JS(marshalJSON(pi.CallGraph))
-	info.CallGraph = htmltemplate.JS(marshalJSON(callGraph))
+	info.AnalysisData = htmltemplate.JS(marshalJSON(pi.Types))
-	info.AnalysisData = htmltemplate.JS(marshalJSON(typeInfos))
 	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("<script type='text/javascript'>document.ANALYSIS_DATA = ")
-		buf.Write(marshalJSON(data))
+		buf.Write(marshalJSON(fi.Data))
 		buf.WriteString(";</script>\n")
 
-		// TODO(adonovan): indicate whether analysis is
+		if status := p.Corpus.Analysis.Status(); status != "" {
-		// disabled, pending, completed or failed.
+			buf.WriteString("<a href='/lib/godoc/analysis/help.html'>Static analysis features</a> ")
-		// For now, display help link only if 'completed'.
+			// TODO(adonovan): show analysis status at per-file granularity.
-		if links != nil {
+			fmt.Fprintf(&buf, "<span style='color: grey'>[%s]</span><br/>", htmlpkg.EscapeString(status))
-			buf.WriteString("<a href='/lib/godoc/analysis/help.html'>Static analysis features</a><br/>")
 		}
 
 		buf.WriteString("<pre>")
-		formatGoSource(&buf, src, links, h, s)
+		formatGoSource(&buf, src, fi.Links, h, s)
 		buf.WriteString("</pre>")
 	} else {
 		buf.WriteString("<pre>")

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.
 
-  <b>Type analysis</b> is relatively quick, requiring just a few seconds for
+  <b>Type analysis</b> is relatively quick, requiring about 10 seconds for
   the &gt;200 packages of the standard library, for example.
 </p>
 
@@ -101,7 +101,7 @@
 
 <h2>Pointer analysis features</h2>
 <p>
-  <code>godoc -analysis=pointer</code> performs a precise
+  <code>godoc -analysis=pointer</code> additionally performs a precise
   whole-program <b>pointer analysis</b>.  In other words, it
   approximates the set of memory locations to which each
   reference&mdash;not just vars of kind <code>*T</code>, but also
@@ -113,10 +113,8 @@
   channel.
 </p>
 <p>
-  <span class='err'>⚠</span> Pointer analysis is currently quite slow,
+  Pointer analysis is slower than type analysis, taking an additional
-  taking around two minutes for the standard library.  This will
+  15 seconds or so for the standard libraries and their tests.
-  improve markedly with the planned addition of a constraint
-  optimizer.
 </p>
 
 <h3>Call graph navigation</h3>
@@ -250,9 +248,6 @@
 
 <h2>Known issues</h2>
 <p>
-  <span class='err'>⚠</span> There is no UI indication of the state of
-  the analysis (pending, complete, failed) during warm-up.</br>
-
   <span class='err'>⚠</span> 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

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.
 
-  <b>Type analysis</b> is relatively quick, requiring just a few seconds for
+  <b>Type analysis</b> is relatively quick, requiring about 10 seconds for
   the >200 packages of the standard library, for example.
 </p>
 
@@ -122,7 +122,7 @@ var Files = map[string]string{
 
 <h2>Pointer analysis features</h2>
 <p>
-  <code>godoc -analysis=pointer</code> performs a precise
+  <code>godoc -analysis=pointer</code> additionally performs a precise
   whole-program <b>pointer analysis</b>.  In other words, it
   approximates the set of memory locations to which each
   reference&mdash;not just vars of kind <code>*T</code>, but also
@@ -134,10 +134,8 @@ var Files = map[string]string{
   channel.
 </p>
 <p>
-  <span class='err'>⚠</span> Pointer analysis is currently quite slow,
+  Pointer analysis is slower than type analysis, taking an additional
-  taking around two minutes for the standard library.  This will
+  15 seconds or so for the standard libraries and their tests.
-  improve markedly with the planned addition of a constraint
-  optimizer.
 </p>
 
 <h3>Call graph navigation</h3>
@@ -271,9 +269,6 @@ var Files = map[string]string{
 
 <h2>Known issues</h2>
 <p>
-  <span class='err'>⚠</span> There is no UI indication of the state of
-  the analysis (pending, complete, failed) during warm-up.</br>
-
   <span class='err'>⚠</span> 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