-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
tools: implement mkcodecache as an executable #27135
Conversation
The idea is that we can decompose mkcodecache out of the node binary so that it's easier to complete the build integration in GYP. |
Test balloon based on #27108 |
@nodejs/build-files @nodejs/process CI is green. can I have some reviews please? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure the split is worth the complexity...
I believe it's worth it, even not for the GYP changes - this is what I would've done in hindsight, see the previous comments in
It is now no longer aware of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved some comments.
I have an idea I hope will simplify this. I'll try to right a suggestion. Anyway, my comments are non-blocking. |
I've split the refactoring into the first commit, and submitted it as #27160 |
295aa35
to
f55c5aa
Compare
Rebased after #27160 landed, and made the progress output optional depending on |
CI is green. Can I have some reviews please? @nodejs/process @nodejs/build-files |
Well I guess we cross approved each other's bits. |
Superseded by #27161 |
This patch splits
NativeModuleLoader
into two parts - a singletonthat only relies on v8 and
node::Mutex
and a proxy class forthe singleton (
NativeModuleEnv
) that provides limited access tothe singleton as well as C++ bindings for the Node.js binary.
A mkcodecache executable is then built on top of the singleton.
This makes it possible to build a Node.js binary with embedded
code cache without building itself using the code cache stub -
the cache is now initialized by
NativeModuleEnv
instead whichcan be refactored out of the mkcodecache dependencies.
Refs: #21563
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes