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.

Sign up for GitHub

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: export cpu_profiler_metadata_size in getHeapCodeStatistics #42818

Merged
merged 1 commit into from
Apr 29, 2022
Merged

v8: export cpu_profiler_metadata_size in getHeapCodeStatistics #42818

merged 1 commit into from
Apr 29, 2022

Conversation

theanarkh
Copy link
Contributor

  • 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++. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels Apr 21, 2022
@theanarkh
Copy link
Contributor Author

@RaisinTen hello, I noticed that getHeapCodeStatistics is also missing a field, so I add the field to it. Can you help review it ? thanks.

RaisinTen
RaisinTen reviewed Apr 24, 2022
View reviewed changes
lib/v8.js Outdated Show resolved Hide resolved
doc/api/v8.md Outdated Show resolved Hide resolved
@theanarkh theanarkh force-pushed the export_field_of_heap_code_statistics branch from 535faee to ad6e8ea Compare April 24, 2022 13:55
RaisinTen
RaisinTen approved these changes Apr 24, 2022
View reviewed changes
Copy link
Contributor

@RaisinTen RaisinTen left a comment

Choose a reason for hiding this comment

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

LGTM

@RaisinTen RaisinTen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 24, 2022
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@github-actions github-actions bot mentioned this pull request Apr 25, 2022
Open
16 tasks
@theanarkh
Copy link
Contributor Author

@RaisinTen Please help run CI again, Thanks. By the way, whether the CI can only be triggered by member of Node.js ?

@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@RaisinTen
Copy link
Contributor

Sure, done! I think CI can be triggered only by triagers and collaborators.

@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@theanarkh
Copy link
Contributor Author

Sure, done! I think CI can be triggered only by triagers and collaborators.

Thanks, i get it.

@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@github-actions github-actions bot mentioned this pull request Apr 26, 2022
Open
14 tasks
@theanarkh
Copy link
Contributor Author

@RaisinTen Hi ! Can this PR be merged ?

@RaisinTen
Copy link
Contributor

Hey, this needs to wait for 7 days before we can merge it, unless someone else also approves it.

@theanarkh
Copy link
Contributor Author

Hey, this needs to wait for 7 days before we can merge it, unless someone else also approves it.

Oh, Thanks.

aduh95
aduh95 reviewed Apr 26, 2022
View reviewed changes
Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

nit

doc/api/v8.md Outdated Show resolved Hide resolved
lib/v8.js Outdated Show resolved Hide resolved
lib/v8.js Outdated Show resolved Hide resolved
@github-actions github-actions bot mentioned this pull request Apr 27, 2022
Open
11 tasks
@theanarkh theanarkh force-pushed the export_field_of_heap_code_statistics branch 2 times, most recently from d4f964e to 4cdc1b1 Compare April 27, 2022 01:41
@aduh95 aduh95 added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Apr 27, 2022
@theanarkh theanarkh force-pushed the export_field_of_heap_code_statistics branch from 4cdc1b1 to 462e028 Compare April 27, 2022 10:53
@theanarkh
Copy link
Contributor Author

@aduh95 Hi, please help review again.

@aduh95
Copy link
Contributor

aduh95 commented Apr 27, 2022

/cc @nodejs/v8

@github-actions github-actions bot mentioned this pull request Apr 28, 2022
Open
18 tasks
@nodejs-github-bot
Copy link
Collaborator

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

@gengjiawen
Copy link
Member

/cc @nodejs/v8

cc @joyeecheung

@github-actions github-actions bot mentioned this pull request Apr 29, 2022
Open
21 tasks
@RaisinTen RaisinTen added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 29, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 29, 2022
@nodejs-github-bot nodejs-github-bot merged commit 000a0c8 into nodejs:master Apr 29, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 000a0c8

targos pushed a commit that referenced this pull request May 2, 2022
@theanarkh @targos
v8: export cpu_profiler_metadata_size in getHeapCodeStatistics
4af0fbd 
PR-URL: #42818
Reviewed-By: Darshan Sen <raisinten@gmail.com>
@targos targos mentioned this pull request May 2, 2022
Merged
@juanarbol
Copy link
Member

This is not supported in the current V8 version in v16.x

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

@aduh95 aduh95 aduh95 left review comments

@RaisinTen RaisinTen RaisinTen approved these changes

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++. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@theanarkh @nodejs-github-bot @RaisinTen @aduh95 @gengjiawen @juanarbol