Merge pull request bcherny#447 from bcherny/perf/optimize-optimizer

Optimize optimizer to avoid deep JSON serialization (fix bcherny#422)

Author: bcherny

Diff for: package.json

@@ -55,7 +55,6 @@
     "glob-promise": "^3.4.0",
     "is-glob": "^4.0.1",
     "json-schema-ref-parser": "^9.0.9",
-    "json-stringify-safe": "^5.0.1",
     "lodash": "^4.17.20",
     "minimist": "^1.2.5",
     "mkdirp": "^1.0.4",

Diff for: src/generator.ts

@@ -1,4 +1,4 @@
-import {omit} from 'lodash'
+import {memoize, omit} from 'lodash'
 import {DEFAULT_OPTIONS, Options} from './index'
 import {
   AST,
@@ -156,7 +156,7 @@ function declareNamedTypes(ast: AST, options: Options, rootASTName: string, proc
   return type
 }
 
-function generateType(ast: AST, options: Options): string {
+function generateTypeUnmemoized(ast: AST, options: Options): string {
   const type = generateRawType(ast, options)
 
   if (options.strictIndexSignatures && ast.keyName === '[k: string]') {
@@ -165,6 +165,7 @@ function generateType(ast: AST, options: Options): string {
 
   return type
 }
+export const generateType = memoize(generateTypeUnmemoized)
 
 function generateRawType(ast: AST, options: Options): string {
   log('magenta', 'generator', ast)

Diff for: src/index.ts

@@ -157,7 +157,7 @@ export async function compile(schema: JSONSchema4, name: string, options: Partia
   const parsed = parse(normalized, _options)
   log('blue', 'parser', time(), '✅ Result:', parsed)
 
-  const optimized = optimize(parsed)
+  const optimized = optimize(parsed, _options)
   if (process.env.VERBOSE) {
     if (isDeepStrictEqual(parsed, optimized)) {
       log('cyan', 'optimizer', time(), '✅ No change')

Diff for: src/optimizer.ts

@@ -1,11 +1,10 @@
-import stringify = require('json-stringify-safe')
 import {uniqBy} from 'lodash'
+import {Options} from '.'
+import {generateType} from './generator'
 import {AST, T_ANY, T_UNKNOWN} from './types/AST'
 import {log} from './utils'
 
-export function optimize(ast: AST, processed = new Set<AST>()): AST {
-  log('cyan', 'optimizer', ast, processed.has(ast) ? '(FROM CACHE)' : '')
-
+export function optimize(ast: AST, options: Options, processed = new Set<AST>()): AST {
   if (processed.has(ast)) {
     return ast
   }
@@ -15,41 +14,61 @@ export function optimize(ast: AST, processed = new Set<AST>()): AST {
   switch (ast.type) {
     case 'INTERFACE':
       return Object.assign(ast, {
-        params: ast.params.map(_ => Object.assign(_, {ast: optimize(_.ast, processed)}))
+        params: ast.params.map(_ => Object.assign(_, {ast: optimize(_.ast, options, processed)}))
       })
     case 'INTERSECTION':
     case 'UNION':
+      // Start with the leaves...
+      const optimizedAST = Object.assign(ast, {
+        params: ast.params.map(_ => optimize(_, options, processed))
+      })
+
       // [A, B, C, Any] -> Any
-      if (ast.params.some(_ => _.type === 'ANY')) {
-        log('cyan', 'optimizer', '[A, B, C, Any] -> Any', ast)
+      if (optimizedAST.params.some(_ => _.type === 'ANY')) {
+        log('cyan', 'optimizer', '[A, B, C, Any] -> Any', optimizedAST)
         return T_ANY
       }
 
       // [A, B, C, Unknown] -> Unknown
-      if (ast.params.some(_ => _.type === 'UNKNOWN')) {
-        log('cyan', 'optimizer', '[A, B, C, Unknown] -> Unknown', ast)
+      if (optimizedAST.params.some(_ => _.type === 'UNKNOWN')) {
+        log('cyan', 'optimizer', '[A, B, C, Unknown] -> Unknown', optimizedAST)
         return T_UNKNOWN
       }
 
+      // [A (named), A] -> [A (named)]
+      if (
+        optimizedAST.params.every(_ => {
+          const a = generateType(omitStandaloneName(_), options)
+          const b = generateType(omitStandaloneName(optimizedAST.params[0]), options)
+          return a === b
+        }) &&
+        optimizedAST.params.some(_ => _.standaloneName !== undefined)
+      ) {
+        log('cyan', 'optimizer', '[A (named), A] -> [A (named)]', optimizedAST)
+        optimizedAST.params = optimizedAST.params.filter(_ => _.standaloneName !== undefined)
+      }
+
       // [A, B, B] -> [A, B]
-      const shouldTakeStandaloneNameIntoAccount = ast.params.filter(_ => _.standaloneName).length > 1
-      const params = uniqBy(
-        ast.params,
-        _ => `
-          ${_.type}-
-          ${shouldTakeStandaloneNameIntoAccount ? _.standaloneName : ''}-
-          ${stringify((_ as any).params)}
-        `
-      )
-      if (params.length !== ast.params.length) {
-        log('cyan', 'optimizer', '[A, B, B] -> [A, B]', ast)
-        ast.params = params
+      const params = uniqBy(optimizedAST.params, _ => generateType(_, options))
+      if (params.length !== optimizedAST.params.length) {
+        log('cyan', 'optimizer', '[A, B, B] -> [A, B]', optimizedAST)
+        optimizedAST.params = params
       }
 
-      return Object.assign(ast, {
-        params: ast.params.map(_ => optimize(_, processed))
+      return Object.assign(optimizedAST, {
+        params: optimizedAST.params.map(_ => optimize(_, options, processed))
       })
     default:
       return ast
   }
 }
+
+// TODO: More clearly disambiguate standalone names vs. aliased names instead.
+function omitStandaloneName<A extends AST>(ast: A): A {
+  switch (ast.type) {
+    case 'ENUM':
+      return ast
+    default:
+      return {...ast, standaloneName: undefined}
+  }
+}