-
Notifications
You must be signed in to change notification settings - Fork 246
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: faster, lazy-friendly runtime loading #4181
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instead of eagerly loading each type from the assemblies in order to annotate the constructos with jsii fully qualified type names (FQNs), leverage the `jsii.rtti` data that is injected by the `jsii` compiler since release `1.19.0`.
RomainMuller
changed the title
feat: leverage runtime type information in jsii runtime
feat: faster, lazy-friendly runtime loading
Jul 17, 2023
# Conflicts: # packages/@jsii/kernel/src/kernel.ts # packages/@jsii/kernel/src/objects.ts
3 tasks
RomainMuller
force-pushed
the
rmuller/lazy-friend
branch
from
July 24, 2023 09:51
8ed8f68
to
8ab3813
Compare
mrgrain
requested changes
Jul 24, 2023
5 tasks
mrgrain
approved these changes
Jul 25, 2023
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
Merging (with squash)... |
rix0rrr
added a commit
that referenced
this pull request
Aug 8, 2023
In #4181, a faster method to load modules was introduced: symlinking instead of recursing through the directory tree, mostly affecting the load times of large modules. Since Windows Vista, non-Administrator users on Windows aren't allowed to create symlinks anymore, so this new loading method fails for users working in corporate Windows environments. Catch the error and fall back to the slower copying method if that happens. Fixes #4208.
mergify bot
pushed a commit
that referenced
this pull request
Aug 8, 2023
In #4181, a faster method to load modules was introduced: symlinking instead of recursing through the directory tree, mostly affecting the load times of large modules. Since Windows Vista, non-Administrator users on Windows aren't allowed to create symlinks anymore, so this new loading method fails for users working in corporate Windows environments. Catch the error and fall back to the slower copying method if that happens. Fixes #4208. --- 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
rix0rrr
added a commit
that referenced
this pull request
Aug 9, 2023
The package cache mechanism that was turned on by default in #4181 works in theory under parallelism, but not in practice. Typically the CDK CLI will prevent CDK apps from running in parallel, but Python testing frameworks like `tox` use subprocess parallelism to speed up test runs, leading to the jsii imports being executed at the same time. Since jsii is sync, the locking needs to be sync. The sync locking feature of the `lockfile` library doesn't have wait support (for good reason), and so when a lock is already acquired by another process it quickly burns through its 12 retries in a hot loop, and then exits with an error. Two changes to address this: - (Ab)use `Atomics.wait()` to get a synchronous sleeping primitive; since `lockSync` doesn't support synchronous sleep, we build our own retry loop with synchronous sleep around `lockSync`. - Since the extracted directory is immutable: if the marker file in the extracted directory exists, we can treat it as evidence that the directory has been completely written and we can skip trying to vy for exclusive access to write it. This avoids all lock contention after the very first CDK app execution. Fixes #4207.
mergify bot
pushed a commit
that referenced
this pull request
Aug 10, 2023
The package cache mechanism that was turned on by default in #4181 works in theory under parallelism, but not in practice. Typically the CDK CLI will prevent CDK apps from running in parallel, but Python testing frameworks like `tox` use subprocess parallelism to speed up test runs, leading to the jsii imports being executed at the same time. Since jsii is sync, the locking needs to be sync. The sync locking feature of the `lockfile` library doesn't have wait support (for good reason), and so when a lock is already acquired by another process it quickly burns through its 12 retries in a hot loop, and then exits with an error. Two changes to address this: - (Ab)use `Atomics.wait()` to get a synchronous sleeping primitive; since `lockSync` doesn't support synchronous sleep, we build our own retry loop with synchronous sleep around `lockSync`. - Since the extracted directory is immutable: if the marker file in the extracted directory exists, we can treat it as evidence that the directory has been completely written and we can skip trying to vy for exclusive access to write it. This avoids all lock contention after the very first CDK app execution. Fixes #4207. --- 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of eagerly loading each type from the assemblies in order to annotate the constructors with jsii fully qualified type names (FQNs), leverage the
jsii.rtti
data that is injected by thejsii
compiler since release1.19.0
.This should make the jsii runtimes friendlier to large libraries that include lazy-loading provisions, such as the
aws-cdk-lib
.In addition to this,
JSII_RUNTIME_PACKAGE_CACHE
was flipped from opt-in to opt-out (set it to any value other thanenabled
to disable), and the@jsii/runtime
entry point now sets--preserve-symlinks
so that we can symbolically link packages from the cache instead of copying them around, which is significantly faster.Finally, the jsii kernel had not opted out of the assembly validation feature, which is redundant in the majority of scenarios, and is quite time-consuming (~500ms for
aws-cdk-lib
)... So also opting out, but allowing users to opt-back-in via an environment variable.Example on a "simple" repro via
aws-cdk-lib
:Before:
After:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.