Skip to content
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

Experimental code cache for built-in js/ts modules #204

Merged
merged 1 commit into from
Dec 16, 2022

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Dec 3, 2022

Builds on #201 which would need to land first

Implements a per-process cache for compiled built-in js/ts modules aimed at reducing the peformance impact of loading those repeatedly across many isolates. This is purely experimental right now so this PR will remain as a draft for the time being.

@jasnell jasnell requested a review from harrishancock December 3, 2022 22:07
@jasnell jasnell force-pushed the jsnell/builtin-modules-experimental-codecache branch from fb042d0 to 4db166c Compare December 3, 2022 22:08
@kentonv
Copy link
Member

kentonv commented Dec 5, 2022

Won't this use unbounded memory as more and more different workers are loaded over time? It seems like we would need some sort of eviction mechanism for these?

@kentonv
Copy link
Member

kentonv commented Dec 5, 2022

Never mind, I didn't get that this is for built-in modules only. This makes tons of sense for built-ins.

@jasnell
Copy link
Member Author

jasnell commented Dec 5, 2022

Yeah, I have no intention of using this for worker bundles for the time being.

@jasnell jasnell force-pushed the jsnell/builtin-modules-external-strings branch from 6ba1d7f to d73efd5 Compare December 6, 2022 19:11
src/workerd/jsg/modules.c++ Outdated Show resolved Hide resolved
@jasnell jasnell force-pushed the jsnell/builtin-modules-experimental-codecache branch from 4db166c to dbd18ce Compare December 7, 2022 15:00
@jasnell jasnell force-pushed the jsnell/builtin-modules-external-strings branch from 983ad7f to a423774 Compare December 9, 2022 19:58
Base automatically changed from jsnell/builtin-modules-external-strings to main December 9, 2022 20:49
@jasnell jasnell force-pushed the jsnell/builtin-modules-experimental-codecache branch from dbd18ce to 8f21c0c Compare December 9, 2022 20:52
@jasnell jasnell marked this pull request as ready for review December 9, 2022 20:52
src/workerd/jsg/modules.c++ Outdated Show resolved Hide resolved
src/workerd/jsg/modules.c++ Outdated Show resolved Hide resolved
src/workerd/jsg/modules.c++ Outdated Show resolved Hide resolved
src/workerd/jsg/modules.h Outdated Show resolved Hide resolved
src/workerd/jsg/modules.h Outdated Show resolved Hide resolved
@jasnell jasnell force-pushed the jsnell/builtin-modules-experimental-codecache branch from ced1492 to 23a4327 Compare December 10, 2022 00:07
src/workerd/jsg/modules.h Outdated Show resolved Hide resolved
Copy link
Member

@kentonv kentonv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM, don't forget to squash fixups.

@jasnell jasnell force-pushed the jsnell/builtin-modules-experimental-codecache branch from 0d2ccfc to 898180d Compare December 16, 2022 22:05
@jasnell jasnell merged commit 8b34faa into main Dec 16, 2022
@jasnell jasnell deleted the jsnell/builtin-modules-experimental-codecache branch December 16, 2022 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants