Filter by types, categories and properties seperately (#39)

* Drop typeTEMP, type filters like `category:type` * Make init a property not a type * Drop `all-core`, use `core`, `all-v8`, `is:init` etc * Set and use excludeKey based on category:type * Catch any `all-v8`s that don't pass isNodeCore() * Update tests * Use d3-fg that has overridable isNodeExcluded() * Set app types to appName * Drop duplicate text in "In Node JS (Node JS)" * Show V8 and deps infobox "In ..." text properly * Fix logic preventing zoom on excluded node * Add checks against selecting excluded node _In theory_ impossible unless someone mangles a hash
clinicjs · Nov 29, 2018 · e186fcc · e186fcc
1 parent 302fa5f
commit e186fcc
Show file tree

Hide file tree

Showing 13 changed files with 159 additions and 125 deletions.
diff --git a/analysis/frame-node.js b/analysis/frame-node.js
@@ -79,20 +79,18 @@ class FrameNode {
     return !getPlatformPath(systemInfo).isAbsolute(fullFileName)
   }
 
-  categorise (systemInfo) {
+  categorise (systemInfo, appName) {
     const { name } = this // this.name remains unmutated: the initial name returned by 0x
 
     const {
       category,
-      type,
-      typeTEMP // Temporary until d3-fg custom property filter complete
-    } = this.getCoreType(name, systemInfo) ||
+      type
+    } = this.getCoreOrV8Type(name, systemInfo) ||
       this.getDepType(name, systemInfo) ||
-      this.getAppType(name, systemInfo)
+      this.getAppType(name, appName)
 
-    this.category = category // Top level filters: 'app' or 'deps' or 'all-core'
+    this.category = category // Top level filters: 'app', 'deps', 'core' or 'all-v8'
     this.type = type // Second-level filters; core are static, app and deps depend on app
-    this.typeTEMP = typeTEMP // Temporary access to dependency name or app directory
 
     if (type === 'regexp') {
       this.formatRegExpName()
@@ -114,19 +112,19 @@ class FrameNode {
     // TODO: add more cases like this
   }
 
-  getCoreType (name, systemInfo) {
+  getCoreOrV8Type (name, systemInfo) {
+    // TODO: see if any subdivisions of core are useful
+    const core = { type: 'core', category: 'core' }
+
     let type
 
-    // TODO: Delete 'init' condition here when adding custom d3-fg filter on properties
-    if (/\[INIT]$/.test(name)) {
-      type = 'init'
-    } else if (/\[CODE:RegExp]$/.test(name)) {
+    if (/\[CODE:RegExp]$/.test(name)) {
       type = 'regexp'
     } else if (!/\.m?js/.test(name)) {
       if (/\[CODE:.*?]$/.test(name) || /v8::internal::.*\[CPP]$/.test(name)) {
         type = 'v8'
       } else /* istanbul ignore next */ if (/\.$/.test(name)) {
-        type = 'core'
+        return core
       } else if (/\[CPP]$/.test(name) || /\[SHARED_LIB]$/.test(name)) {
         type = 'cpp'
       } else if (/\[eval]/.test(name)) {
@@ -141,12 +139,12 @@ class FrameNode {
     } else if (/ native /.test(name)) {
       type = 'native'
     } else if (this.isNodeCore(systemInfo)) {
-      type = 'core'
+      return core
     }
 
     return type ? {
       type,
-      category: 'all-core'
+      category: 'all-v8'
     } : null
   }
 
@@ -158,31 +156,22 @@ class FrameNode {
 
     const match = name.match(depDirRegex)
     return match ? {
-      // TODO: use this type after adding custom d3-fg filter on properties including category
-      typeTEMP: match[1],
-      type: 'deps', // Temporary until d3-fg custom property filter complete
+      type: match[1],
       category: 'deps'
     } : null
   }
 
-  getAppType (name, systemInfo) {
-    const platformPath = getPlatformPath(systemInfo)
-
-    const parentDir = platformPath.join(systemInfo.mainDirectory, `..${systemInfo.pathSeparator}`)
-
+  getAppType (name, appName) {
     return {
-      // TODO: use this type after adding custom d3-fg filter on properties including category
-      typeTEMP: platformPath.relative(parentDir, platformPath.dirname(this.fileName)),
-      type: 'app', // Temporary until d3-fg custom property filter complete
+      // TODO: profile some large applications with a lot of app code, see if there's a useful heuristic to split
+      // out types, e.g. folders containing more than n files or look for common patterns like `lib`
+      type: appName,
       category: 'app'
     }
   }
 
   anonymise (systemInfo) {
-    if (!this.fileName || this.isNodeCore(systemInfo) ||
-        // Init frames are in the all-core category but may be part of the app.
-        // TODO Remove the `init` after adding custom d3-fg filter on properties like `isInit`
-        (this.category === 'all-core' && this.type !== 'init')) {
+    if (!this.fileName || this.isNodeCore(systemInfo) || this.category === 'all-v8') {
       return
     }
 
@@ -236,7 +225,6 @@ class FrameNode {
       target: this.target || '',
 
       type: this.type,
-      typeTEMP: this.typeTEMP, // Temporary until d3-fg custom property filter complete
       category: this.category,
 
       isOptimised: this.isOptimised,

diff --git a/analysis/index.js b/analysis/index.js
@@ -37,10 +37,19 @@ async function analyse (paths) {
       childrenVisibilityToggle: true }
   ]
 
+  codeAreas.forEach(area => {
+    area.excludeKey = area.id
+    if (area.children) {
+      area.children.forEach(childArea => {
+        childArea.excludeKey = `${area.id}:${childArea.id}`
+      })
+    }
+  })
+
   const steps = [
     (tree) => labelNodes(tree),
     (tree) => tree.walk((node) => {
-      node.categorise(systemInfo)
+      node.categorise(systemInfo, appName)
       node.format(systemInfo)
     }),
     (tree) => setStackTop(tree, defaultExclude)

diff --git a/package.json b/package.json
@@ -23,7 +23,7 @@
     "browserify-inline-svg": "^1.0.2",
     "copy-to-clipboard": "^3.0.8",
     "d3-array": "^2.0.2",
-    "d3-fg": "^6.11.1",
+    "d3-fg": "^6.12.0",
     "d3-selection": "^1.3.2",
     "deepmerge": "^2.1.1",
     "flame-gradient": "^1.0.0",

diff --git a/shared.js b/shared.js
@@ -18,21 +18,23 @@ function setStackTop (node, exclude = this.exclude) {
 }
 
 function isNodeExcluded (node, exclude = this.exclude) {
-  if (node.isInit && exclude.has('init')) return true
-  if (node.isInlinable && exclude.has('inlinable')) return true
+  if (node.isInit && exclude.has('is:init')) return true
+  if (node.isInlinable && exclude.has('is:inlinable')) return true
 
   if (exclude.has(node.category)) return true
-  if (exclude.has(node.type)) return true
+
+  // Namespace types by category in case someone installs a dependency named 'cpp' etc
+  if (exclude.has(`${node.category}:${node.type}`)) return true
 
   return false
 }
 
 const defaultExclude = new Set([
-  'v8',
-  'cpp',
-  'init',
-  'native',
-  'regexp'
+  'all-v8:v8',
+  'all-v8:cpp',
+  'all-v8:native',
+  'all-v8:regexp',
+  'is:init'
 ])
 
 module.exports = {

diff --git a/test/analysis-categorise.test.js b/test/analysis-categorise.test.js
@@ -17,29 +17,51 @@ const windows = {
   nodeVersions: { node: '8.13.0' }
 }
 
-function byProps (properties, sysinfo) {
+function byProps (properties, sysinfo, appName = 'some-app') {
   const node = new FrameNode(properties)
-  node.categorise(sysinfo)
+  node.categorise(sysinfo, appName)
   return node
 }
 
 test('analysis - categorise node names', (t) => {
-  function byName (name, sysinfo) {
-    const { type } = byProps({ name }, sysinfo)
+  function byName (name, sysinfo, appName) {
+    const { type } = byProps({ name }, sysinfo, appName)
     return type
   }
 
-  t.equal(byName('NativeModule.compile internal/bootstrap/loaders.js:236:44 [INIT]', linux), 'init')
-  t.equal(byName('~getMediaTypePriority /root/0x/examples/rest-api/node_modules/negotiator/lib/mediaType.js:99:30 [INLINABLE]', linux), 'deps')
+  t.match(byProps({ name: 'NativeModule.compile internal/bootstrap/loaders.js:236:44 [INIT]' }, linux), {
+    category: 'core',
+    type: 'core',
+    isInit: true,
+    isInlinable: false
+  })
+  t.match(byProps({ name: '~(anonymous) /home/username/.npm/prefix/lib/node_modules/clinic/node_modules/0x/lib/preload/no-cluster.js:1:11 [INIT]' }, linux), {
+    category: 'deps',
+    type: 'clinic',
+    isInit: true,
+    isInlinable: false
+  })
+  t.match(byProps({ name: '~getMediaTypePriority /root/0x/examples/rest-api/node_modules/negotiator/lib/mediaType.js:99:30 [INLINABLE]' }, linux), {
+    category: 'deps',
+    type: 'negotiator',
+    isInit: false,
+    isInlinable: true
+  })
+  t.match(byProps({ name: '~walk /home/username/dash-ast/index.js:26:15 [INLINABLE]' }, linux, 'dash-ast'), {
+    category: 'app',
+    type: 'dash-ast',
+    isInit: false,
+    isInlinable: true
+  })
+
   t.equal(byName('/usr/bin/node [SHARED_LIB]', linux), 'cpp')
   t.equal(byName('C:\\Program Files\\nodejs\\node.exe [SHARED_LIB]', windows), 'cpp')
   t.equal(byName('v8::internal::Runtime_CompileLazy(int, v8::internal::Object**, v8::internal::Isolate*) [CPP]', linux), 'v8')
   t.equal(byName('Call_ReceiverIsNotNullOrUndefined [CODE:Builtin]', linux), 'v8')
-  t.equal(byName('ArrayFilter [CODE:Builtin] [INIT]'), 'init')
   t.equal(byName('NativeModule.require internal/bootstrap/loaders.js:140:34', linux), 'core')
-  t.equal(byName('_run /root/0x/examples/rest-api/node_modules/restify/lib/server.js:807:38', linux), 'deps')
-  t.equal(byName('(anonymous) /root/0x/examples/rest-api/etag.js:1:11', linux), 'app')
-  t.equal(byName('(anonymous) C:\\Documents\\Contains spaces\\0x\\examples\\rest-api\\etag.js:1:11', windows), 'app')
+  t.equal(byName('_run /root/0x/examples/rest-api/node_modules/restify/lib/server.js:807:38', linux), 'restify')
+  t.equal(byName('(anonymous) /root/0x/examples/rest-api/etag.js:1:11', linux, 'rest-api'), 'rest-api')
+  t.equal(byName('(anonymous) C:\\Documents\\Contains spaces\\0x\\examples\\rest-api\\etag.js:1:11', windows), 'some-app')
   t.equal(byName('InnerArraySort native array.js:486:24', linux), 'native')
   t.equal(byName('[\u0000zA-Z\u0000#$%&\'*+.|~]+$ [CODE:RegExp]', linux), 'regexp')
 
@@ -58,12 +80,12 @@ test('analysis - categorise node properties', (t) => {
   // Handle multiple unexpected custom flags
   const customNode = byProps({
     name: '~Unexpected multiple customFlags C:\\Documents\\Contains spaces\\sub_dir\\index.js:1:1'
-  }, windows)
+  }, windows, 'Contains spaces')
 
   customNode.format(windows)
 
   t.equal(customNode.category, 'app')
-  t.equal(customNode.typeTEMP, 'Contains spaces\\sub_dir')
+  t.equal(customNode.type, 'Contains spaces')
   t.equal(customNode.functionName, 'Unexpected multiple customFlags')
   t.equal(customNode.fileName, '.\\sub_dir\\index.js')
   t.ok(customNode.isOptimised)
@@ -77,7 +99,7 @@ test('analysis - categorise node properties', (t) => {
   depNode.format(windows)
 
   t.equal(depNode.category, 'deps')
-  t.equal(depNode.typeTEMP, 'some-module')
+  t.equal(depNode.type, 'some-module')
   t.equal(depNode.functionName, 'Funcname')
   t.equal(depNode.fileName, '.\\node_modules\\some-module\\index.js')
   t.equal(depNode.lineNumber, 1)
@@ -95,7 +117,7 @@ test('analysis - categorise node properties', (t) => {
 
   regexpNode.format(windows)
 
-  t.equal(regexpNode.category, 'all-core')
+  t.equal(regexpNode.category, 'all-v8')
   t.equal(regexpNode.type, 'regexp')
   t.equal(regexpNode.functionName, '/[\u0000zA-Z\u0000#$%&\'*+.|~]+$/')
   t.equal(regexpNode.fileName, '[CODE:RegExp]')
@@ -118,7 +140,7 @@ test('analysis - categorise node properties', (t) => {
   t.notOk(inlinableNode.isOptimised)
   t.notOk(inlinableNode.isOptimisable)
   t.notOk(dataTree.isNodeExcluded(inlinableNode))
-  dataTree.exclude.add('inlinable')
+  dataTree.exclude.add('is:inlinable')
   t.ok(dataTree.isNodeExcluded(inlinableNode))
 
   const initNode = byProps({

diff --git a/visualizer/data-tree.js b/visualizer/data-tree.js
@@ -20,7 +20,7 @@ class DataTree {
 
     this.useMerged = true
     this.showOptimizationStatus = false
-    this.exclude = new Set(['cpp', 'regexp', 'v8', 'native', 'init'])
+    this.exclude = shared.defaultExclude
 
     // Set and updated in .update()
     this.flatByHottest = null
@@ -111,12 +111,12 @@ class DataTree {
   }
 
   getFlattenedSorted (sorter, arr) {
-    const filtered = arr.filter(node => !this.exclude.has(node.type))
+    const filtered = arr.filter(node => !this.isNodeExcluded(node))
     return filtered.sort(sorter)
   }
 
   getHeatColor (node, arr = this.flatByHottest) {
-    if (!node) return flameGradient(0)
+    if (!node || this.isNodeExcluded(node)) return flameGradient(0)
 
     const pivotPoint = this.mean / (this.mean + this.maxRootAboveMean + this.maxRootBelowMean)
 
@@ -145,30 +145,28 @@ class DataTree {
   }
 
   getFilteredStackSorter () {
-    const exclude = this.exclude
-
-    function getValue (node) {
-      if (exclude.has(node.type)) {
-        // Value of hidden frames is the sum of their visible children
-        return node.children ? node.children.reduce((acc, child) => {
-          return acc + getValue(child)
-        }, 0) : 0
-      }
+    return (nodeA, nodeB) => {
+      const valueA = this.getNodeValue(nodeA)
+      const valueB = this.getNodeValue(nodeB)
 
-      // d3-fg sets `value` to 0 to hide off-screen nodes.
-      // there's no other property to indicate this but the original value is stored on `.original`.
-      if (node.value === 0 && typeof node.original === 'number') {
-        return node.original
-      }
-      return node.value
+      return valueA === valueB ? 0 : valueA > valueB ? -1 : 1
     }
+  }
 
-    return (nodeA, nodeB) => {
-      const valueA = getValue(nodeA)
-      const valueB = getValue(nodeB)
+  getNodeValue (node) {
+    if (this.isNodeExcluded(node)) {
+      // Value of hidden frames is the sum of their visible children
+      return node.children ? node.children.reduce((acc, child) => {
+        return acc + this.getNodeValue(child)
+      }, 0) : 0
+    }
 
-      return valueA === valueB ? 0 : valueA > valueB ? -1 : 1
+    // d3-fg sets `value` to 0 to hide off-screen nodes.
+    // there's no other property to indicate this but the original value is stored on `.original`.
+    if (node.value === 0 && typeof node.original === 'number') {
+      return node.original
     }
+    return node.value
   }
 
   getSortPosition (node, arr = this.flatByHottest) {

diff --git a/visualizer/flame-graph.js b/visualizer/flame-graph.js
@@ -132,6 +132,9 @@ class FlameGraph extends HtmlContent {
     this.flameGraph = d3Fg({
       tree: dataTree.activeTree(),
       exclude: dataTree.exclude,
+      isNodeExcluded: node => {
+        return this.ui.dataTree.isNodeExcluded(node.data)
+      },
       element: this.d3Chart.node(),
       cellHeight: this.cellHeight,
       collapseHiddenNodeWidths: true,

diff --git a/visualizer/history.js b/visualizer/history.js
@@ -27,12 +27,12 @@ class History extends EventEmitter {
       'app',
       'deps',
       'core',
-      'native',
-      'cpp',
-      'v8',
-      'regexp',
-      'init',
-      'inlinable'
+      'all-v8:native',
+      'all-v8:cpp',
+      'all-v8:v8',
+      'all-v8:regexp',
+      'is:init',
+      'is:inlinable'
     ]
 
     if (window.location.hash) {

diff --git a/visualizer/info-box.js b/visualizer/info-box.js
@@ -41,11 +41,11 @@ class InfoBox extends HtmlContent {
     this.pathText = node.fileName
     this.rankNumber = this.ui.dataTree.getSortPosition(node)
 
-    // 'typeTEMP' key is temporary until d3-fg custom filter is complete
-    const typeLabel = this.ui.getLabelFromKey(node.typeTEMP || node.type, true)
+    const typeLabel = node.category === 'core' ? '' : ` (${this.ui.getLabelFromKey(`${node.category}:${node.type}`, true)})`
     const categoryLabel = this.ui.getLabelFromKey(node.category, true)
-    this.areaText = `In ${categoryLabel} (${typeLabel})`
+    this.areaText = `In ${categoryLabel}${typeLabel}`
 
+    if (node.isInit) this.areaText += '. In initialization process'
     if (node.isInlinable) this.areaText += '. Inlinable'
     if (node.isOptimisable) this.areaText += '. Optimizable'
     if (node.isOptimised) this.areaText += '. Is optimized'