X Tutup
The Wayback Machine - https://web.archive.org/web/20230131200320/https://github.com/nodejs/node/pull/46255
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

Jump to bottom

v8: support gc profile #46255

Merged
merged 1 commit into from Jan 25, 2023
Merged

v8: support gc profile #46255

merged 1 commit into from Jan 25, 2023

Conversation

theanarkh
Copy link
Contributor

support gc profile. the content is as follows.

{
  "version": 1,
  "startTime": 1674059033862,
  "statistics": [
    {
      "gcType": "Scavenge",
      "beforeGC": {
        "heapStatistics": {
          "totalHeapSize": 5005312,
          "totalHeapSizeExecutable": 524288,
          "totalPhysicalSize": 5226496,
          "totalAvailableSize": 4341325216,
          "totalGlobalHandlesSize": 8192,
          "usedGlobalHandlesSize": 2112,
          "usedHeapSize": 4883840,
          "heapSizeLimit": 4345298944,
          "mallocedMemory": 254128,
          "externalMemory": 225138,
          "peakMallocedMemory": 181760
        },
        "heapSpaceStatistics": [
          {
            "spaceName": "read_only_space",
            "spaceSize": 0,
            "spaceUsedSize": 0,
            "spaceAvailableSize": 0,
            "physicalSpaceSize": 0
          }
        ]
      },
      "cost": 1574.14,
      "afterGC": {
        "heapStatistics": {
          "totalHeapSize": 6053888,
          "totalHeapSizeExecutable": 524288,
          "totalPhysicalSize": 5500928,
          "totalAvailableSize": 4341101384,
          "totalGlobalHandlesSize": 8192,
          "usedGlobalHandlesSize": 2112,
          "usedHeapSize": 4059096,
          "heapSizeLimit": 4345298944,
          "mallocedMemory": 254128,
          "externalMemory": 225138,
          "peakMallocedMemory": 181760
        },
        "heapSpaceStatistics": [
          {
            "spaceName": "read_only_space",
            "spaceSize": 0,
            "spaceUsedSize": 0,
            "spaceAvailableSize": 0,
            "physicalSpaceSize": 0
          }
        ]
      }
    }
  ],
  "endtTime": 1674059036865
}
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@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 Jan 18, 2023
addaleax
addaleax reviewed Jan 18, 2023
View changes
src/node_v8.cc Outdated Show resolved Hide resolved
@theanarkh theanarkh force-pushed the support_gc_profile branch 2 times, most recently from cbcd60d to 8a8e086 Compare Jan 18, 2023
anonrig
anonrig reviewed Jan 18, 2023
View changes
doc/api/v8.md Outdated Show resolved Hide resolved
lib/v8.js Outdated Show resolved Hide resolved
src/node_v8.cc Outdated Show resolved Hide resolved
src/node_v8.h Outdated Show resolved Hide resolved
src/node_v8.cc Outdated Show resolved Hide resolved
@theanarkh theanarkh force-pushed the support_gc_profile branch 5 times, most recently from 4756504 to e57ab16 Compare Jan 19, 2023
addaleax
addaleax approved these changes Jan 19, 2023
View changes
src/node_v8.cc Outdated Show resolved Hide resolved
@addaleax addaleax added the semver-minor PRs that contain new features and should be released in the next minor version. label Jan 19, 2023
@theanarkh theanarkh force-pushed the support_gc_profile branch 2 times, most recently from ce8dfe7 to ca62c3b Compare Jan 19, 2023
VoltrexKeyva
VoltrexKeyva reviewed Jan 19, 2023
View changes
doc/api/v8.md Outdated Show resolved Hide resolved
doc/api/v8.md Outdated Show resolved Hide resolved
lib/v8.js Outdated Show resolved Hide resolved
src/node_v8.cc Outdated Show resolved Hide resolved
src/node_v8.cc Outdated Show resolved Hide resolved
test/common/v8.js Outdated Show resolved Hide resolved
test/parallel/test-v8-take-gc-profile-exit-before-stop.js Outdated Show resolved Hide resolved
test/parallel/test-v8-take-gc-profile-in-worker.js Outdated Show resolved Hide resolved
test/parallel/test-v8-take-gc-profile.js Outdated Show resolved Hide resolved
jasnell
jasnell reviewed Jan 20, 2023
View changes
doc/api/v8.md Outdated Show resolved Hide resolved
jasnell
jasnell reviewed Jan 20, 2023
View changes
doc/api/v8.md Outdated Show resolved Hide resolved
jasnell
jasnell reviewed Jan 20, 2023
View changes
doc/api/v8.md Outdated Show resolved Hide resolved
@jasnell
Copy link
Member

jasnell commented Jan 20, 2023

The perf_hooks module already has a PerformanceEntry type for GC operations. I'm wondering if it would make more sense to add this additional detail to that existing API than to add an entirely new API.

@theanarkh
Copy link
Contributor Author

The perf_hooks module already has a PerformanceEntry type for GC operations. I'm wondering if it would make more sense to add this additional detail to that existing API than to add an entirely new API.

Every time a GC occurs, perf_hooks will create a new entry and call JS callback later, I'm not sure If this will cause performance problems when GC is frequent, so i think we can implement this in C++. And we can add more informations in this API in the furure, like process.report do, such as GC load.

Currently i think the user can implement GC profile in JS land by combine perf_hooks, v8.getHeapSpaceStatistics() , and v8.getHeapStatistics(). i also try to implement GC load by perf_hooks(nodejs-gc-load), but i am going to implement this in Node.js Core.

@theanarkh theanarkh added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 21, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 21, 2023
@nodejs-github-bot
Copy link
Contributor

CI: https://ci.nodejs.org/job/node-test-pull-request/49119/

@theanarkh theanarkh requested review from addaleax and jasnell Jan 22, 2023
@theanarkh theanarkh added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 24, 2023
@theanarkh
Copy link
Contributor Author

theanarkh commented Jan 24, 2023 •
edited

@addaleax @jasnell Hi, can help review this PR again ? thanks.

addaleax
addaleax reviewed Jan 24, 2023
View changes
src/node_v8.cc Outdated Show resolved Hide resolved
addaleax
addaleax reviewed Jan 24, 2023
View changes
lib/v8.js Outdated Show resolved Hide resolved
RafaelGSS
RafaelGSS approved these changes Jan 24, 2023
View changes
doc/api/v8.md Outdated Show resolved Hide resolved
@theanarkh theanarkh force-pushed the support_gc_profile branch 2 times, most recently from d6b457b to 50a83ff Compare Jan 24, 2023
@theanarkh theanarkh added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 25, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 25, 2023
@nodejs-github-bot
Copy link
Contributor

CI: https://ci.nodejs.org/job/node-test-pull-request/49159/

@nodejs-github-bot
Copy link
Contributor

CI: https://ci.nodejs.org/job/node-test-pull-request/49165/

@theanarkh theanarkh added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 25, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 25, 2023
@nodejs-github-bot nodejs-github-bot merged commit 733ef4e into nodejs:main Jan 25, 2023
51 checks passed
@nodejs-github-bot
Copy link
Contributor

Landed in 733ef4e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anonrig anonrig

@VoltrexKeyva VoltrexKeyva

@fhinkel fhinkel

@addaleax addaleax

@RafaelGSS RafaelGSS

@jasnell jasnell

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@theanarkh @jasnell @nodejs-github-bot @fhinkel @addaleax @anonrig @RafaelGSS @VoltrexKeyva
X Tutup