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

src: track ShadowRealm native objects correctly in the heap snapshot #47389

Merged
merged 1 commit into from
Apr 6, 2023

Conversation

joyeecheung
Copy link
Member

Previously the native ShadowRealm objects were never actually tracked in the heap snapshot - the tracking starts with the Environment, we don't call the tracking methods unless it's reachable natively from the Environment. This patch adds a set in the Environment for tracking shadow realms in the heap snapshot.

Drive-by: remove redundant MemoryInfo() overrides from the derived classes of Realm and remove the tracking of env from Realm::MemoryInfo() because the realms don't hold the Environment alive (even for the principal realm, it's the other way around).

Previously the native ShadowRealm objects were never actually tracked
in the heap snapshot - the tracking starts with the Environment,
we don't call the tracking methods unless it's reachable natively from
the Environment. This patch adds a set in the Environment for tracking
shadow realms in the heap snapshot.

Drive-by: remove redundant MemoryInfo() overrides from the derived
classes of Realm and remove the tracking of `env` from
`Realm::MemoryInfo()` because the realms don't hold the Environment
alive (even for the principal realm, it's the other way around).
@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 Apr 3, 2023
@legendecas legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 4, 2023
@legendecas
Copy link
Member

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 4, 2023
@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 Apr 5, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 6, 2023
@nodejs-github-bot nodejs-github-bot merged commit 583dcca into nodejs:main Apr 6, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 583dcca

RafaelGSS pushed a commit that referenced this pull request Apr 13, 2023
Previously the native ShadowRealm objects were never actually tracked
in the heap snapshot - the tracking starts with the Environment,
we don't call the tracking methods unless it's reachable natively from
the Environment. This patch adds a set in the Environment for tracking
shadow realms in the heap snapshot.

Drive-by: remove redundant MemoryInfo() overrides from the derived
classes of Realm and remove the tracking of `env` from
`Realm::MemoryInfo()` because the realms don't hold the Environment
alive (even for the principal realm, it's the other way around).

PR-URL: #47389
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@danielleadams danielleadams added the backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. 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.

6 participants