Skip to content

Commit

Permalink
chore: make jsii fully synchronous (#3467)
Browse files Browse the repository at this point in the history
With a sample set of 1, I have detemined that there is about 30% tax
caused by asynchronous functions in the compiler. The `typescript`
compiler itself is fully synchronous, and we basically moved to do the
same, and this turns out reducing the amount of time wasted in the
runloop and in promise bookeeping.

Tested on `aws-cdk-lib`, it turns out to be reducing the compile time
from ~142 seconds to ~100 seconds.



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
  • Loading branch information
RomainMuller authored Apr 4, 2022
1 parent 8da74aa commit 2be1cde
Show file tree
Hide file tree
Showing 47 changed files with 608 additions and 724 deletions.
3 changes: 2 additions & 1 deletion packages/@jsii/python-runtime/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ __pycache__
.mypy_cache
.pytest_cache

build
# Occasionally left behind by pip, transient (safe to delete)
build/

README.md
6 changes: 6 additions & 0 deletions packages/@jsii/python-runtime/build-tools/deps.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#!/usr/bin/env npx ts-node

import { removeSync } from 'fs-extra';
import { join, resolve } from 'path';
import { venv, runCommand } from './_constants';

Expand All @@ -18,3 +19,8 @@ runCommand(
['-m', 'pip', 'install', '-r', resolve(__dirname, '..', 'requirements.txt')],
{ env },
);

// Sometimes, pip leaves a `build` directory behind, which can mess with mypy
// when run with `pytest --mypy`. This directory should be transient and so we
// simply clean it up here.
removeSync(resolve(__dirname, '..', 'build'));
9 changes: 4 additions & 5 deletions packages/@jsii/python-runtime/pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
[build-system]
requires = [
"pip~=20.2",
"setuptools~=50.3",
"wheel~=0.35",
]
requires = ["pip~=20.2", "setuptools~=50.3", "wheel~=0.35"]
build-backend = 'setuptools.build_meta'

[tool.black]
target-version = ['py36', 'py37', 'py38']
include = '\.pyi?$'
exclude = '\.(git|mypy_cache|env)'

[tool.mypy]
ignore_missing_imports = true
6 changes: 3 additions & 3 deletions packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -1128,10 +1128,10 @@
"primitive": "number"
},
{
"fqn": "jsii-calc.Multiply"
"fqn": "@scope/jsii-calc-lib.Number"
},
{
"fqn": "@scope/jsii-calc-lib.Number"
"fqn": "jsii-calc.Multiply"
}
]
}
Expand Down Expand Up @@ -16931,5 +16931,5 @@
}
},
"version": "3.20.120",
"fingerprint": "sqJBfFAp4Hg5OgntWB3IRyRS7mpPF7G3qoh4NYSDeSw="
"fingerprint": "x8U5PHtkMg1Me8BU5rwy98qvqY6p+AhernFr4narjHc="
}
28 changes: 14 additions & 14 deletions packages/jsii-diff/test/diagnostics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import { classifyDiagnostics, hasErrors } from '../lib/diagnostics';
import { compare } from './util';

// ----------------------------------------------------------------------
test('experimental elements lead to warnings', async () => {
const mms = await compare(
test('experimental elements lead to warnings', () => {
const mms = compare(
`
/** @experimental */
export class Foo1 { }
Expand All @@ -21,8 +21,8 @@ test('experimental elements lead to warnings', async () => {
});

// ----------------------------------------------------------------------
test('external stability violations are reported as warnings', async () => {
const mms = await compare(
test('external stability violations are reported as warnings', () => {
const mms = compare(
`
/** @stability external */
export class Foo1 { }
Expand All @@ -40,8 +40,8 @@ test('external stability violations are reported as warnings', async () => {
});

// ----------------------------------------------------------------------
test('warnings can be turned into errors', async () => {
const mms = await compare(
test('warnings can be turned into errors', () => {
const mms = compare(
`
/** @experimental */
export class Foo1 { }
Expand All @@ -59,8 +59,8 @@ test('warnings can be turned into errors', async () => {
});

// ----------------------------------------------------------------------
test('external stability violations are never turned into errors', async () => {
const mms = await compare(
test('external stability violations are never turned into errors', () => {
const mms = compare(
`
/** @stability external */
export class Foo1 { }
Expand All @@ -78,8 +78,8 @@ test('external stability violations are never turned into errors', async () => {
});

// ----------------------------------------------------------------------
test('errors can be skipped', async () => {
const mms = await compare(
test('errors can be skipped', () => {
const mms = compare(
`
export class Foo1 { }
`,
Expand All @@ -100,8 +100,8 @@ test('errors can be skipped', async () => {
});

// ----------------------------------------------------------------------
test('changing stable to experimental is breaking', async () => {
const mms = await compare(
test('changing stable to experimental is breaking', () => {
const mms = compare(
`
/** @stable */
export class Foo1 { }
Expand All @@ -128,8 +128,8 @@ test('changing stable to experimental is breaking', async () => {

// ----------------------------------------------------------------------

test('can make fields optional in output struct if it is marked @external', async () => {
const mms = await compare(
test('can make fields optional in output struct if it is marked @external', () => {
const mms = compare(
`
/** @stability external */
export interface TheStruct {
Expand Down
25 changes: 8 additions & 17 deletions packages/jsii-diff/test/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,25 @@ import * as reflect from 'jsii-reflect';
import { compareAssemblies } from '../lib';
import { Mismatches } from '../lib/types';

export async function expectNoError(original: string, updated: string) {
const mms = await compare(original, updated);
export function expectNoError(original: string, updated: string) {
const mms = compare(original, updated);
for (const msg of mms.messages()) {
console.error(`- ${msg}`);
}
expect(mms.count).toBe(0);
}

export async function expectError(
export function expectError(
error: RegExp | undefined,
original: string,
updated: string,
) {
if (error == null) {
await expectNoError(original, updated);
expectNoError(original, updated);
return;
}

const mms = await compare(original, updated);
const mms = compare(original, updated);
expect(mms.count).not.toBe(0);

const msgs = Array.from(mms.messages());
Expand All @@ -32,23 +32,14 @@ export async function expectError(
}
}

export async function compare(
original: string,
updated: string,
): Promise<Mismatches> {
export function compare(original: string, updated: string): Mismatches {
const ass1 = sourceToAssemblyHelper(original);
await expect(ass1).resolves.not.toThrowError();
const ts1 = new reflect.TypeSystem();
const originalAssembly = ts1.addAssembly(
new reflect.Assembly(ts1, await ass1),
);
const originalAssembly = ts1.addAssembly(new reflect.Assembly(ts1, ass1));

const ass2 = sourceToAssemblyHelper(updated);
await expect(ass2).resolves.not.toThrowError();
const ts2 = new reflect.TypeSystem();
const updatedAssembly = ts2.addAssembly(
new reflect.Assembly(ts2, await ass2),
);
const updatedAssembly = ts2.addAssembly(new reflect.Assembly(ts2, ass2));

return compareAssemblies(originalAssembly, updatedAssembly);
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/jsii-pacmak/test/generated-code/examples.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ for (const name of fs.readdirSync(EXAMPLES_ROOT)) {
const file = path.join(EXAMPLES_ROOT, name);
test(name, async () => {
const source = await fs.readFile(file, 'utf8');
const compiled = await jsii.compileJsiiForTest(source);
const compiled = jsii.compileJsiiForTest(source);

const targets: AssemblyTargets = {
dotnet: {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/jsii-reflect/test/__snapshots__/tree.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions packages/jsii-reflect/test/independent.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import * as reflect from '../lib';
import { assemblyFromSource } from './util';

test('get full github source location for a class or method', async () => {
test('get full github source location for a class or method', () => {
// WHEN
const assembly = await assemblyFromSource(
const assembly = assemblyFromSource(
`
export class Foo {
public bar() {
Expand Down
32 changes: 16 additions & 16 deletions packages/jsii-reflect/test/type-system.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ test('Submodules can have a README', () => {
);
});

test('overridden member knows about both parent types', async () => {
const ts = await typeSystemFromSource(`
test('overridden member knows about both parent types', () => {
const ts = typeSystemFromSource(`
export class Foo {
public bar() {
Array.isArray(3);
Expand Down Expand Up @@ -256,8 +256,8 @@ describe('Stability', () => {
// ----------------------------------------------------------------------

describe('lowest stability guarantee is advertised', () => {
test('when subclass is experimental', async () => {
const ts = await typeSystemFromSource(`
test('when subclass is experimental', () => {
const ts = typeSystemFromSource(`
/**
* @stable
*/
Expand Down Expand Up @@ -285,8 +285,8 @@ describe('Stability', () => {
expect(method.docs.stability).toEqual(Stability.Experimental);
});

test('when method is experimental', async () => {
const ts = await typeSystemFromSource(`
test('when method is experimental', () => {
const ts = typeSystemFromSource(`
/**
* @stable
*/
Expand Down Expand Up @@ -321,8 +321,8 @@ describe('Stability', () => {
expect(method.docs.stability).toEqual(Stability.Experimental);
});

test('when method is explicitly marked stable', async () => {
const ts = await typeSystemFromSource(`
test('when method is explicitly marked stable', () => {
const ts = typeSystemFromSource(`
/**
* @stable
*/
Expand Down Expand Up @@ -357,8 +357,8 @@ describe('Stability', () => {
expect(method.docs.stability).toEqual(Stability.Experimental);
});

test('external stability', async () => {
const ts = await typeSystemFromSource(`
test('external stability', () => {
const ts = typeSystemFromSource(`
/**
* @stability external
*/
Expand All @@ -383,8 +383,8 @@ describe('Stability', () => {
});
});

test('TypeSystem.properties', async () => {
const ts = await typeSystemFromSource(`
test('TypeSystem.properties', () => {
const ts = typeSystemFromSource(`
export namespace submodule {
export class Foo {
public readonly test = 'TEST';
Expand All @@ -397,8 +397,8 @@ test('TypeSystem.properties', async () => {
expect(ts.properties).toHaveLength(2);
});

test('TypeSystem.methods', async () => {
const ts = await typeSystemFromSource(`
test('TypeSystem.methods', () => {
const ts = typeSystemFromSource(`
export namespace submodule {
export class Foo {
public method(): void {}
Expand All @@ -411,8 +411,8 @@ test('TypeSystem.methods', async () => {
expect(ts.methods).toHaveLength(2);
});

test('Assembly allTypes includes submodule types', async () => {
const asm = await assemblyFromSource({
test('Assembly allTypes includes submodule types', () => {
const asm = assemblyFromSource({
'index.ts': 'export * as submod from "./submod";',
'submod.ts': `export class Foo {}`,
});
Expand Down
10 changes: 5 additions & 5 deletions packages/jsii-reflect/test/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,19 @@ import { sourceToAssemblyHelper, MultipleSourceFiles, PackageInfo } from 'jsii';

import { Assembly, TypeSystem } from '../lib';

export async function typeSystemFromSource(
export function typeSystemFromSource(
source: string | MultipleSourceFiles,
cb?: (obj: PackageInfo) => void,
) {
const asm = await assemblyFromSource(source, cb);
const asm = assemblyFromSource(source, cb);
return asm.system;
}

export async function assemblyFromSource(
export function assemblyFromSource(
source: string | MultipleSourceFiles,
cb?: (obj: PackageInfo) => void,
): Promise<Assembly> {
const ass = await sourceToAssemblyHelper(source, cb);
): Assembly {
const ass = sourceToAssemblyHelper(source, cb);
const ts = new TypeSystem();
return ts.addAssembly(new Assembly(ts, ass));
}
Loading

0 comments on commit 2be1cde

Please sign in to comment.