Skip to content

Commit 3a6b21c

Browse files
rix0rrrrix0rrr
authored
fix(jsii-reflect): don't load same assembly multiple times (#461)
deCDK tests were calling `loadModule()` for every package. `loadModule()` did have some recursion avoidance INSIDE one call, but not across multiple load calls on the same type system. This brings down the deCDK tests from 80s to 10s. Add an option to disable validation, which would bring the full CDK typesystem loading down from 10s to 600ms (validation is enabled by default). Some more type exposure updates so that we can update awslint and friends in the CDK repository to take advantage of the new jsii model. Also: change some JVM settings in pacmak to speed up Java build from around 30s to around 10s per package (on my machine, on my Oracle JVM, YYMV on OpenJDK).
1 parent c81b4c1 commit 3a6b21c

File tree

13 files changed

+196
-25
lines changed

13 files changed

+196
-25
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ async function loadFromFilesystem(name: string) {
9191
if (stat.isDirectory()) {
9292
return await ts.loadModule(name);
9393
} else {
94-
return await ts.loadFile(name, true);
94+
return await ts.loadFile(name);
9595
}
9696
}
9797

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import process = require('process');
77
import yargs = require('yargs');
88
import logging = require('../lib/logging');
99
import { Target } from '../lib/target';
10+
import { Timers } from '../lib/timer';
1011
import { resolveDependencyDirectory, shell } from '../lib/util';
1112
import { VERSION_DESC } from '../lib/version';
1213

@@ -128,22 +129,34 @@ import { VERSION_DESC } from '../lib/version';
128129
await updateNpmIgnore(packageDir, npmIgnoreExclude);
129130
}
130131

132+
const timers = new Timers();
133+
131134
const tmpdir = await fs.mkdtemp(path.join(os.tmpdir(), 'npm-pack'));
132135
try {
133-
const tarball = await npmPack(packageDir, tmpdir);
136+
const tarball = await timers.recordAsync('npm pack', () => {
137+
return npmPack(packageDir, tmpdir);
138+
});
134139
for (const targetName of targets) {
135140
// if we are targeting a single language, output to outdir, otherwise outdir/<target>
136141
const targetOutputDir = (targets.length > 1 || forceSubdirectory)
137142
? path.join(outDir, targetName.toString())
138143
: outDir;
139144
logging.debug(`Building ${pkg.name}/${targetName}: ${targetOutputDir}`);
140-
await generateTarget(packageDir, targetName.toString(), targetOutputDir, tarball);
145+
146+
await timers.recordAsync(targetName.toString(), () =>
147+
generateTarget(packageDir, targetName.toString(), targetOutputDir, tarball)
148+
);
141149
}
142150
} finally {
143-
logging.debug(`Removing ${tmpdir}`);
151+
if (argv.clean) {
152+
logging.debug(`Removing ${tmpdir}`);
153+
} else {
154+
logging.debug(`Temporary directory retained (--no-clean): ${tmpdir}`);
155+
}
144156
await fs.remove(tmpdir);
145157
}
146158

159+
logging.info(`Packaged. ${timers.display()}`);
147160
}
148161

149162
async function generateTarget(packageDir: string, targetName: string, targetOutputDir: string, tarball: string) {
@@ -221,7 +234,7 @@ async function updateNpmIgnore(packageDir: string, excludeOutdir: string | undef
221234

222235
if (modified) {
223236
await fs.writeFile(npmIgnorePath, lines.join('\n') + '\n');
224-
logging.info('Updated .npmignre');
237+
logging.info('Updated .npmignore');
225238
}
226239

227240
function includePattern(comment: string, ...patterns: string[]) {

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,14 @@ export default class Java extends Target {
6565
await shell(
6666
'mvn',
6767
[...mvnArguments, 'deploy', `-D=altDeploymentRepository=local::default::${url}`, `--settings=${userXml}`],
68-
{ cwd: sourceDir }
68+
{
69+
cwd: sourceDir,
70+
env: {
71+
// Twiddle the JVM settings a little for Maven. Delaying JIT compilation
72+
// brings down Maven execution time by about 1/3rd (15->10s, 30->20s)
73+
MAVEN_OPTS: `${process.env.MAVEN_OPTS || ''} -XX:+TieredCompilation -XX:TieredStopAtLevel=1`
74+
}
75+
}
6976
);
7077
}
7178

@@ -433,7 +440,10 @@ class JavaGenerator extends Generator {
433440
},
434441
configuration: {
435442
failOnError: false,
436-
show: 'protected'
443+
show: 'protected',
444+
// Adding these makes JavaDoc generation about a 3rd faster (which is far and away the most
445+
// expensive part of the build)
446+
additionalJOption: ['-J-XX:+TieredCompilation</additionalJOption', '-J-XX:TieredStopAtLevel=1</additionalJOption']
437447
}
438448
}]
439449
}

‎packages/jsii-pacmak/lib/timer.ts

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/**
2+
* A single timer
3+
*/
4+
export class Timer {
5+
public timeMs?: number;
6+
private startTime: number;
7+
8+
constructor(public readonly label: string) {
9+
this.startTime = Date.now();
10+
}
11+
12+
public start() {
13+
this.startTime = Date.now();
14+
}
15+
16+
public end() {
17+
this.timeMs = (Date.now() - this.startTime) / 1000;
18+
}
19+
20+
public isSet() {
21+
return this.timeMs !== undefined;
22+
}
23+
24+
public humanTime() {
25+
if (!this.timeMs) { return '???'; }
26+
27+
const parts = [];
28+
29+
let time = this.timeMs;
30+
if (time > 60) {
31+
const mins = Math.floor(time / 60);
32+
parts.push(mins + 'm');
33+
time -= mins * 60;
34+
}
35+
parts.push(time.toFixed(1) + 's');
36+
37+
return parts.join('');
38+
}
39+
}
40+
41+
/**
42+
* A collection of Timers
43+
*/
44+
export class Timers {
45+
private readonly timers: Timer[] = [];
46+
47+
public record<T>(label: string, operation: () => T): T {
48+
const timer = this.start(label);
49+
try {
50+
const x = operation();
51+
timer.end();
52+
return x;
53+
} catch (e) {
54+
timer.end();
55+
throw e;
56+
}
57+
}
58+
59+
public async recordAsync<T>(label: string, operation: () => Promise<T>) {
60+
const timer = this.start(label);
61+
try {
62+
const x = await operation();
63+
timer.end();
64+
return x;
65+
} catch (e) {
66+
timer.end();
67+
throw e;
68+
}
69+
}
70+
71+
public start(label: string) {
72+
const timer = new Timer(label);
73+
this.timers.push(timer);
74+
return timer;
75+
}
76+
77+
public display(): string {
78+
const timers = this.timers.filter(t => t.isSet());
79+
timers.sort((a: Timer, b: Timer) => b.timeMs! - a.timeMs!);
80+
return timers.map(t => `${t.label} (${t.humanTime()})`).join(' | ');
81+
}
82+
}

‎packages/jsii-pacmak/lib/util.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ export function resolveDependencyDirectory(packageDir: string, dependencyName: s
1717

1818
export function shell(cmd: string, args: string[], options: SpawnOptions): Promise<string> {
1919
return new Promise<string>((resolve, reject) => {
20-
const child = spawn(cmd, args, { ...options, shell: true, stdio: ['ignore', 'pipe', 'pipe'] });
20+
logging.debug(cmd, args.join(' '), JSON.stringify(options));
21+
const child = spawn(cmd, args, { ...options, shell: true, env: { ...process.env, ...options.env || {} }, stdio: ['ignore', 'pipe', 'pipe'] });
2122
const stdout = new Array<Buffer>();
2223
const stderr = new Array<Buffer>();
2324
child.stdout.on('data', chunk => {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@
103103
<configuration>
104104
<failOnError>false</failOnError>
105105
<show>protected</show>
106+
<additionalJOption>-J-XX:+TieredCompilation&lt;/additionalJOption</additionalJOption>
107+
<additionalJOption>-J-XX:TieredStopAtLevel=1&lt;/additionalJOption</additionalJOption>
106108
</configuration>
107109
</plugin>
108110
</plugins>

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@
103103
<configuration>
104104
<failOnError>false</failOnError>
105105
<show>protected</show>
106+
<additionalJOption>-J-XX:+TieredCompilation&lt;/additionalJOption</additionalJOption>
107+
<additionalJOption>-J-XX:TieredStopAtLevel=1&lt;/additionalJOption</additionalJOption>
106108
</configuration>
107109
</plugin>
108110
</plugins>

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,8 @@
134134
<configuration>
135135
<failOnError>false</failOnError>
136136
<show>protected</show>
137+
<additionalJOption>-J-XX:+TieredCompilation&lt;/additionalJOption</additionalJOption>
138+
<additionalJOption>-J-XX:TieredStopAtLevel=1&lt;/additionalJOption</additionalJOption>
137139
</configuration>
138140
</plugin>
139141
</plugins>

‎packages/jsii-reflect/lib/assembly.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,16 @@ export class Assembly {
154154
return this._types[fqn];
155155
}
156156

157+
/**
158+
* Validate an assembly after loading
159+
*
160+
* If the assembly was loaded without validation, call this to validate
161+
* it after all. Throws an exception if validation fails.
162+
*/
163+
public validate() {
164+
jsii.validateAssembly(this.spec);
165+
}
166+
157167
private get _dependencies() {
158168
if (!this._dependencyCache) {
159169
this._dependencyCache = { };

‎packages/jsii-reflect/lib/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
export * from './assembly';
22
export * from './class';
3+
export * from './callable';
34
export * from './dependency';
45
export * from './docs';
56
export * from './enum';

0 commit comments

Comments
 (0)