Skip to content

Conversation

joyeecheung
Copy link
Member

Previously the ContextifyContext holds a MicrotaskQueueWrap which in turns holds a MicrotaskQueue in a shared pointer. The indirection is actually unnecessary, we can directly hold the MicrotaskQueue via a unique pointer in ContextifyContext, the lifetime would still remain the same but the graph would be simpler, and this removes the additional JS -> C++ to create the wrapper object.

Previously the ContextifyContext holds a MicrotaskQueueWrap which in
turns holds a MicrotaskQueue in a shared pointer. The indirection is
actually unnecessary, we can directly hold the MicrotaskQueue via
a unique pointer in ContextifyContext, the lifetime would still
remain the same but the graph would be simpler, and this removes
the additional JS -> C++ to create the wrapper object.
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/modules
  • @nodejs/vm

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Aug 1, 2023
@joyeecheung
Copy link
Member Author

@nodejs/vm @nodejs/modules Can I have some reviews please?

@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 15, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 15, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung joyeecheung added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 16, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 16, 2023
@nodejs-github-bot nodejs-github-bot merged commit b4a2be4 into nodejs:main Aug 16, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in b4a2be4

UlisesGascon pushed a commit that referenced this pull request Sep 10, 2023
Previously the ContextifyContext holds a MicrotaskQueueWrap which in
turns holds a MicrotaskQueue in a shared pointer. The indirection is
actually unnecessary, we can directly hold the MicrotaskQueue via
a unique pointer in ContextifyContext, the lifetime would still
remain the same but the graph would be simpler, and this removes
the additional JS -> C++ to create the wrapper object.

PR-URL: #48982
Reviewed-By: Stephen Belanger <[email protected]>
@UlisesGascon UlisesGascon mentioned this pull request Sep 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants