Merge branch 'fix-perf' into release-2.10.1 (fixes #1058)

Author: benmosher

CHANGELOG.md

@@ -11,6 +11,9 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 - Fixer for [`first`] ([#1046], thanks [@fengkfengk])
 - `allow-require` option for [`no-commonjs`] rule ([#880], thanks [@futpib])
 
+### Fixed
+- memory/CPU regression where ASTs were held in memory ([#1058], thanks [@klimashkin]/[@lukeapage])
+
 ## [2.10.0] - 2018-03-29
 ### Added
 - Autofixer for [`order`] rule ([#711], thanks [@tihonove])
@@ -530,6 +533,7 @@ for info on changes for earlier releases.
 [#314]: https://github.com/benmosher/eslint-plugin-import/pull/314
 [#912]: https://github.com/benmosher/eslint-plugin-import/pull/912
 
+[#1058]: https://github.com/benmosher/eslint-plugin-import/issues/1058
 [#886]: https://github.com/benmosher/eslint-plugin-import/issues/886
 [#863]: https://github.com/benmosher/eslint-plugin-import/issues/863
 [#842]: https://github.com/benmosher/eslint-plugin-import/issues/842
@@ -702,3 +706,5 @@ for info on changes for earlier releases.
 [@danny-andrews]: https://github.com/dany-andrews
 [@fengkfengk]: https://github.com/fengkfengk
 [@futpib]: https://github.com/futpib
+[@klimashkin]: https://github.com/klimashkin
+[@lukeapage]: https://github.com/lukeapage

src/ExportMap.js

@@ -265,20 +265,14 @@ ExportMap.get = function (source, context) {
   const path = resolve(source, context)
   if (path == null) return null
 
-  return ExportMap.for(path, context)
+  return ExportMap.for(childContext(path, context))
 }
 
-ExportMap.for = function (path, context) {
-  let exportMap
+ExportMap.for = function (context) {
+  const { path } = context
 
-  const cacheKey = hashObject({
-    settings: context.settings,
-    parserPath: context.parserPath,
-    parserOptions: context.parserOptions,
-    path,
-  }).digest('hex')
-
-  exportMap = exportCache.get(cacheKey)
+  const cacheKey = hashObject(context).digest('hex')
+  let exportMap = exportCache.get(cacheKey)
 
   // return cached ignore
   if (exportMap === null) return null
@@ -307,6 +301,7 @@ ExportMap.for = function (path, context) {
     return null
   }
 
+  log('cache miss', cacheKey, 'for path', path)
   exportMap = ExportMap.parse(path, content, context)
 
   // ambiguous modules return null
@@ -355,14 +350,14 @@ ExportMap.parse = function (path, content, context) {
 
   const namespaces = new Map()
 
-  function remotePath(node) {
-    return resolve.relative(node.source.value, path, context.settings)
+  function remotePath(value) {
+    return resolve.relative(value, path, context.settings)
   }
 
-  function resolveImport(node) {
-    const rp = remotePath(node)
+  function resolveImport(value) {
+    const rp = remotePath(value)
     if (rp == null) return null
-    return ExportMap.for(rp, context)
+    return ExportMap.for(childContext(rp, context))
   }
 
   function getNamespace(identifier) {
@@ -385,12 +380,20 @@ ExportMap.parse = function (path, content, context) {
   function captureDependency(declaration) {
     if (declaration.source == null) return null
 
-    const p = remotePath(declaration)
-    if (p == null || m.imports.has(p)) return p
-
-    const getter = () => ExportMap.for(p, context)
-    m.imports.set(p, { getter, source: declaration.source })
-    return p
+    const p = remotePath(declaration.source.value)
+    if (p == null) return null
+    const existing = m.imports.get(p)
+    if (existing != null) return existing.getter
+
+    const getter = () => ExportMap.for(childContext(p, context))
+    m.imports.set(p, {
+      getter,
+      source: {  // capturing actual node reference holds full AST in memory!
+        value: declaration.source.value,
+        loc: declaration.source.loc,
+      },
+    })
+    return getter
   }
 
 
@@ -406,8 +409,8 @@ ExportMap.parse = function (path, content, context) {
     }
 
     if (n.type === 'ExportAllDeclaration') {
-      const p = captureDependency(n)
-      if (p) m.dependencies.add(m.imports.get(p).getter)
+      const getter = captureDependency(n)
+      if (getter) m.dependencies.add(getter)
       return
     }
 
@@ -416,7 +419,7 @@ ExportMap.parse = function (path, content, context) {
       captureDependency(n)
       let ns
       if (n.specifiers.some(s => s.type === 'ImportNamespaceSpecifier' && (ns = s))) {
-        namespaces.set(ns.local.name, n)
+        namespaces.set(ns.local.name, n.source.value)
       }
       return
     }
@@ -443,6 +446,7 @@ ExportMap.parse = function (path, content, context) {
         }
       }
 
+      const nsource = n.source && n.source.value
       n.specifiers.forEach((s) => {
         const exportMeta = {}
         let local
@@ -454,7 +458,7 @@ ExportMap.parse = function (path, content, context) {
             break
           case 'ExportNamespaceSpecifier':
             m.namespace.set(s.exported.name, Object.defineProperty(exportMeta, 'namespace', {
-              get() { return resolveImport(n) },
+              get() { return resolveImport(nsource) },
             }))
             return
           case 'ExportSpecifier':
@@ -469,7 +473,7 @@ ExportMap.parse = function (path, content, context) {
         }
 
         // todo: JSDoc
-        m.reexports.set(s.exported.name, { local, getImport: () => resolveImport(n) })
+        m.reexports.set(s.exported.name, { local, getImport: () => resolveImport(nsource) })
       })
     }
   })
@@ -505,3 +509,16 @@ export function recursivePatternCapture(pattern, callback) {
       break
   }
 }
+
+/**
+ * don't hold full context object in memory, just grab what we need.
+ */
+function childContext(path, context) {
+  const { settings, parserOptions, parserPath } = context
+  return {
+    settings,
+    parserOptions,
+    parserPath,
+    path,
+  }
+}