fix(jsii-reflect): don't load same assembly multiple times

packages/jsii-diff/bin/jsii-diff.ts

@@ -91,7 +91,7 @@ async function loadFromFilesystem(name: string) {
   if (stat.isDirectory()) {
     return await ts.loadModule(name);
   } else {
-    return await ts.loadFile(name, true);
+    return await ts.loadFile(name);
   }
 }
 

packages/jsii-pacmak/bin/jsii-pacmak.ts

@@ -7,6 +7,7 @@ import process = require('process');
 import yargs = require('yargs');
 import logging = require('../lib/logging');
 import { Target } from '../lib/target';
+import { Timers } from '../lib/timer';
 import { resolveDependencyDirectory, shell } from '../lib/util';
 import { VERSION_DESC } from '../lib/version';
 
@@ -128,22 +129,34 @@ import { VERSION_DESC } from '../lib/version';
             await updateNpmIgnore(packageDir, npmIgnoreExclude);
         }
 
+        const timers = new Timers();
+
         const tmpdir = await fs.mkdtemp(path.join(os.tmpdir(), 'npm-pack'));
         try {
-            const tarball = await npmPack(packageDir, tmpdir);
+            const tarball = await timers.recordAsync('npm pack', () => {
+                return npmPack(packageDir, tmpdir);
+            });
             for (const targetName of targets) {
                 // if we are targeting a single language, output to outdir, otherwise outdir/<target>
                 const targetOutputDir = (targets.length > 1 || forceSubdirectory)
                                       ? path.join(outDir, targetName.toString())
                                       : outDir;
                 logging.debug(`Building ${pkg.name}/${targetName}: ${targetOutputDir}`);
-                await generateTarget(packageDir, targetName.toString(), targetOutputDir, tarball);
+
+                await timers.recordAsync(targetName.toString(), () =>
+                    generateTarget(packageDir, targetName.toString(), targetOutputDir, tarball)
+                );
             }
         } finally {
-            logging.debug(`Removing ${tmpdir}`);
+            if (argv.clean) {
+                logging.debug(`Removing ${tmpdir}`);
+            } else {
+                logging.debug(`Temporary directory retained (--no-clean): ${tmpdir}`);
+            }
             await fs.remove(tmpdir);
         }
 
+        logging.info(`Packaged. ${timers.display()}`);
     }
 
     async function generateTarget(packageDir: string, targetName: string, targetOutputDir: string, tarball: string) {
@@ -221,7 +234,7 @@ async function updateNpmIgnore(packageDir: string, excludeOutdir: string | undef
 
     if (modified) {
         await fs.writeFile(npmIgnorePath, lines.join('\n') + '\n');
-        logging.info('Updated .npmignre');
+        logging.info('Updated .npmignore');
     }
 
     function includePattern(comment: string, ...patterns: string[]) {

packages/jsii-pacmak/lib/targets/java.ts

@@ -65,7 +65,14 @@ export default class Java extends Target {
         await shell(
             'mvn',
             [...mvnArguments, 'deploy', `-D=altDeploymentRepository=local::default::${url}`, `--settings=${userXml}`],
-            { cwd: sourceDir }
+            {
+                cwd: sourceDir,
+                env: {
+                    // Twiddle the JVM settings a little for Maven. Delaying JIT compilation
+                    // brings down Maven execution time by about 1/3rd (15->10s, 30->20s)
+                    MAVEN_OPTS: `${process.env.MAVEN_OPTS || ''} -XX:+TieredCompilation -XX:TieredStopAtLevel=1`
+                }
+             }
         );
     }
 
@@ -433,7 +440,10 @@ class JavaGenerator extends Generator {
                                 },
                                 configuration: {
                                     failOnError: false,
-                                    show: 'protected'
+                                    show: 'protected',
+                                    // Adding these makes JavaDoc generation about a 3rd faster (which is far and away the most
+                                    // expensive part of the build)
+                                    additionalJOption: ['-J-XX:+TieredCompilation</additionalJOption', '-J-XX:TieredStopAtLevel=1</additionalJOption']
                                 }
                             }]
                         }

packages/jsii-pacmak/lib/timer.ts

@@ -0,0 +1,82 @@
+/**
+ * A single timer
+ */
+export class Timer {
+  public timeMs?: number;
+  private startTime: number;
+
+  constructor(public readonly label: string) {
+    this.startTime = Date.now();
+  }
+
+  public start() {
+    this.startTime = Date.now();
+  }
+
+  public end() {
+    this.timeMs = (Date.now() - this.startTime) / 1000;
+  }
+
+  public isSet() {
+    return this.timeMs !== undefined;
+  }
+
+  public humanTime() {
+    if (!this.timeMs) { return '???'; }
+
+    const parts = [];
+
+    let time = this.timeMs;
+    if (time > 60) {
+      const mins = Math.floor(time / 60);
+      parts.push(mins + 'm');
+      time -= mins * 60;
+    }
+    parts.push(time.toFixed(1) + 's');
+
+    return parts.join('');
+  }
+}
+
+/**
+ * A collection of Timers
+ */
+export class Timers {
+  private readonly timers: Timer[] = [];
+
+  public record<T>(label: string, operation: () => T): T {
+    const timer = this.start(label);
+    try {
+      const x = operation();
+      timer.end();
+      return x;
+    } catch (e) {
+      timer.end();
+      throw e;
+    }
+  }
+
+  public async recordAsync<T>(label: string, operation: () => Promise<T>) {
+    const timer = this.start(label);
+    try {
+      const x = await operation();
+      timer.end();
+      return x;
+    } catch (e) {
+      timer.end();
+      throw e;
+    }
+  }
+
+  public start(label: string) {
+    const timer = new Timer(label);
+    this.timers.push(timer);
+    return timer;
+  }
+
+  public display(): string {
+    const timers = this.timers.filter(t => t.isSet());
+    timers.sort((a: Timer, b: Timer) => b.timeMs! - a.timeMs!);
+    return timers.map(t => `${t.label} (${t.humanTime()})`).join(' | ');
+  }
+}

packages/jsii-pacmak/lib/util.ts

@@ -17,7 +17,8 @@ export function resolveDependencyDirectory(packageDir: string, dependencyName: s
 
 export function shell(cmd: string, args: string[], options: SpawnOptions): Promise<string> {
     return new Promise<string>((resolve, reject) => {
-        const child = spawn(cmd, args, { ...options, shell: true, stdio: ['ignore', 'pipe', 'pipe'] });
+        logging.debug(cmd, args.join(' '), JSON.stringify(options));
+        const child = spawn(cmd, args, { ...options, shell: true, env: { ...process.env, ...options.env || {} }, stdio: ['ignore', 'pipe', 'pipe'] });
         const stdout = new Array<Buffer>();
         const stderr = new Array<Buffer>();
         child.stdout.on('data', chunk => {

packages/jsii-pacmak/test/expected.jsii-calc-base/java/pom.xml

@@ -103,6 +103,8 @@
         <configuration>
           <failOnError>false</failOnError>
           <show>protected</show>
+          <additionalJOption>-J-XX:+TieredCompilation&lt;/additionalJOption</additionalJOption>
+          <additionalJOption>-J-XX:TieredStopAtLevel=1&lt;/additionalJOption</additionalJOption>
         </configuration>
       </plugin>
     </plugins>

packages/jsii-pacmak/test/expected.jsii-calc-lib/java/pom.xml

@@ -103,6 +103,8 @@
         <configuration>
           <failOnError>false</failOnError>
           <show>protected</show>
+          <additionalJOption>-J-XX:+TieredCompilation&lt;/additionalJOption</additionalJOption>
+          <additionalJOption>-J-XX:TieredStopAtLevel=1&lt;/additionalJOption</additionalJOption>
         </configuration>
       </plugin>
     </plugins>

packages/jsii-pacmak/test/expected.jsii-calc/java/pom.xml

@@ -134,6 +134,8 @@
         <configuration>
           <failOnError>false</failOnError>
           <show>protected</show>
+          <additionalJOption>-J-XX:+TieredCompilation&lt;/additionalJOption</additionalJOption>
+          <additionalJOption>-J-XX:TieredStopAtLevel=1&lt;/additionalJOption</additionalJOption>
         </configuration>
       </plugin>
     </plugins>

packages/jsii-reflect/lib/assembly.ts

@@ -154,6 +154,16 @@ export class Assembly {
     return this._types[fqn];
   }
 
+  /**
+   * Validate an assembly after loading
+   *
+   * If the assembly was loaded without validation, call this to validate
+   * it after all. Throws an exception if validation fails.
+   */
+  public validate() {
+    jsii.validateAssembly(this.spec);
+  }
+
   private get _dependencies() {
     if (!this._dependencyCache) {
       this._dependencyCache = { };

packages/jsii-reflect/lib/index.ts

@@ -1,5 +1,6 @@
 export * from './assembly';
 export * from './class';
+export * from './callable';
 export * from './dependency';
 export * from './docs';
 export * from './enum';

packages/jsii-reflect/lib/initializer.ts

@@ -5,6 +5,10 @@ import { SourceLocatable } from './source';
 import { MemberKind, TypeMember } from './type-member';
 
 export class Initializer extends Callable implements Documentable, Overridable, TypeMember, SourceLocatable {
+  public static isInitializer(x: Callable): x is Initializer {
+    return x instanceof Initializer;
+  }
+
   public readonly kind = MemberKind.Initializer;
   public readonly name = '<initializer>';
   public readonly abstract = false;

packages/jsii-reflect/lib/method.ts

@@ -15,6 +15,10 @@ import { TypeSystem } from './type-system';
 export const INITIALIZER_NAME = '<initializer>';
 
 export class Method extends Callable implements Documentable, Overridable, TypeMember, SourceLocatable {
+  public static isMethod(x: Callable): x is Method {
+    return x instanceof Method;
+  }
+
   public readonly kind = MemberKind.Method;
 
   constructor(

packages/jsii-reflect/lib/type-system.ts

@@ -32,18 +32,21 @@ export class TypeSystem {
    * If `fileOrDirectory` is a file, it will be treated as a single .jsii file.
    * If `fileOrDirectory` is a directory, it will be treated as a jsii npm module.
    *
+   * Not validating makes the difference between loading assemblies with lots
+   * of dependencies (such as app-delivery) in 90ms vs 3500ms.
+   *
    * @param fileOrDirectory A .jsii file path or a module directory
+   * @param validate Whether or not to validate the assembly while loading it.
    */
-  public async load(fileOrDirectory: string) {
+  public async load(fileOrDirectory: string, options: { validate?: boolean } = {}) {
     if ((await stat(fileOrDirectory)).isDirectory()) {
-      return await this.loadModule(fileOrDirectory);
+      return await this.loadModule(fileOrDirectory, options);
     } else {
-      return await this.loadFile(fileOrDirectory);
+      return await this.loadFile(fileOrDirectory, { ...options, isRoot: true });
     }
   }
 
-  public async loadModule(dir: string): Promise<Assembly> {
-    const visited = new Set<string>();
+  public async loadModule(dir: string, options: { validate?: boolean } = {}): Promise<Assembly> {
     const self = this;
 
     const out = await _loadModule(dir, true);
@@ -54,18 +57,34 @@ export class TypeSystem {
     return out;
 
     async function _loadModule(moduleDirectory: string, isRoot = false) {
-      if (visited.has(moduleDirectory)) {
-        return;
-      }
-      visited.add(moduleDirectory);
-
       const filePath = path.join(moduleDirectory, 'package.json');
       const pkg = JSON.parse((await readFile(filePath)).toString());
       if (!pkg.jsii) {
         throw new Error(`No "jsii" section in ${filePath}`);
       }
 
-      const root = await self.loadFile(path.join(moduleDirectory, '.jsii'), isRoot);
+      // Load the assembly, but don't recurse if we already have an assembly with the same name.
+      // Validation is not an insignificant time sink, and loading IS insignificant, so do a
+      // load without validation first. This saves about 2/3rds of processing time.
+      const asm = await self.loadAssembly(path.join(moduleDirectory, '.jsii'), false);
+      if (self.includesAssembly(asm.name)) {
+        const existing = self.findAssembly(asm.name);
+        if (existing.version !== asm.version) {
+          throw new Error(`Conflicting versions of ${asm.name} in type system: previously loaded ${existing.version}, trying to load ${asm.version}`);
+        }
+        // Make sure that we mark this thing as root after all if it wasn't yet.
+        if (isRoot) {
+          self.addRoot(asm);
+        }
+
+        return existing;
+      }
+
+      if (options.validate !== false) {
+        asm.validate();
+      }
+
+      const root = await self.addAssembly(asm, { isRoot });
       const bundled: string[] = pkg.bundledDependencies || pkg.bundleDependencies || [];
 
       const loadDependencies = async (deps: { [name: string]: string }) => {
@@ -87,12 +106,12 @@ export class TypeSystem {
     }
   }
 
-  public async loadFile(file: string, isRoot = true) {
-    const spec = JSON.parse((await readFile(file)).toString());
-    return this.addAssembly(new Assembly(this, jsii.validateAssembly(spec)), isRoot);
+  public async loadFile(file: string, options: { isRoot?: boolean, validate?: boolean } = {}) {
+    const assembly = await this.loadAssembly(file, options.validate !== false);
+    return this.addAssembly(assembly, options);
   }
 
-  public addAssembly(asm: Assembly, isRoot = true) {
+  public addAssembly(asm: Assembly, options: { isRoot?: boolean } = {}) {
     if (asm.system !== this) {
       throw new Error('Assembly has been created for different typesystem');
     }
@@ -102,8 +121,8 @@ export class TypeSystem {
       this.assemblies.push(asm);
     }
 
-    if (isRoot && !this.roots.includes(asm)) {
-      this.roots.push(asm);
+    if (options.isRoot !== false) {
+      this.addRoot(asm);
     }
 
     return asm;
@@ -118,6 +137,10 @@ export class TypeSystem {
     return name in this._assemblyLookup;
   }
 
+  public isRoot(name: string) {
+    return this.roots.map(r => r.name).includes(name);
+  }
+
   public findAssembly(name: string) {
     if (!(name in this._assemblyLookup)) {
       throw new Error(`Assembly "${name}" not found`);
@@ -205,4 +228,21 @@ export class TypeSystem {
     });
     return out;
   }
+
+  /**
+   * Load an assembly without adding it to the typesystem
+   * @param file Assembly file to load
+   * @param validate Whether to validate the assembly or just assume it matches the schema
+   */
+  private async loadAssembly(file: string, validate = true) {
+    const spec = JSON.parse((await readFile(file)).toString());
+    const ass = validate ? jsii.validateAssembly(spec) : spec as jsii.Assembly;
+    return new Assembly(this, ass);
+  }
+
+  private addRoot(asm: Assembly) {
+    if (!this.roots.map(r => r.name).includes(asm.name)) {
+      this.roots.push(asm);
+    }
+  }
 }