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

deps: V8: backport 76c3ac5 from upstream #18298

Merged
merged 1 commit into from
Jan 24, 2018
Merged

deps: V8: backport 76c3ac5 from upstream #18298

merged 1 commit into from
Jan 24, 2018

Conversation

ofrobots
Copy link
Contributor

@ofrobots ofrobots commented Jan 22, 2018 •
edited
Loading

This fixes a bug in the CPU profiler where some ticks were attributed
to the wrong file.

This affects all supported versions. The commit here should cherry-pick rather easily to older version; modulo the change to the version bump in v8-version.h rather than common.gypi.

Original commit message:
[cpu-profiler] Fix script name when recording inlining info

Use the script name from the shared function info to create an
inline entry. Otherwise functions are attributed to the wrong file
in the CpuProfileNode.

See googleapis/cloud-profiler-nodejs#89

Bug: v8:7203, v8:7241
Change-Id: I8ea31943741770e6611275a9c93375922b934547
Reviewed-on: https://chromium-review.googlesource.com/848093
Reviewed-by: Jaroslav Sevcik jarin@chromium.org
Commit-Queue: Franziska Hinkelmann franzih@chromium.org
Cr-Commit-Position: refs/heads/master@{#50339}

Refs: v8/v8@76c3ac5

/cc @nodejs/v8

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

deps: v8

CI: https://ci.nodejs.org/job/node-test-pull-request/12663/
V8-CI: https://ci.nodejs.org/view/All/job/node-test-commit-v8-linux/1172/

@ofrobots ofrobots added v8 engine Issues and PRs related to the V8 dependency. v4.x labels Jan 22, 2018
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency. labels Jan 22, 2018
@ofrobots ofrobots added the v9.x label Jan 22, 2018
@ofrobots
Copy link
Contributor Author

In case #17489 lands first, this PR will need to be re-targeted to 9.x. Upstream merged the fix into V8 6.4.

@ofrobots ofrobots mentioned this pull request Jan 22, 2018
Closed
@ofrobots
deps: V8: backport 76c3ac5 from upstream
00687fb 
This fixes a bug in the CPU profiler where some ticks were attributed
to the wrong file.

Original commit message:
  [cpu-profiler] Fix script name when recording inlining info

  Use the script name from the shared function info to create an
  inline entry. Otherwise functions are attributed to the wrong file
  in the CpuProfileNode.

  See googleapis/cloud-profiler-nodejs#89

  Bug: v8:7203, v8:7241
  Change-Id: I8ea31943741770e6611275a9c93375922b934547
  Reviewed-on: https://chromium-review.googlesource.com/848093
  Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
  Commit-Queue: Franziska Hinkelmann <franzih@chromium.org>
  Cr-Commit-Position: refs/heads/master@{nodejs#50339}

Refs: v8/v8@76c3ac5
PR-URL: nodejs#18298
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@ofrobots
Copy link
Contributor Author

Landed in 00687fb.

@ofrobots ofrobots merged commit 00687fb into nodejs:master Jan 24, 2018
@ofrobots ofrobots deleted the backport-v8-7241 branch January 24, 2018 18:39
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
@ofrobots @MylesBorins
deps: V8: backport 76c3ac5 from upstream
e2315ca 
This fixes a bug in the CPU profiler where some ticks were attributed
to the wrong file.

Original commit message:
  [cpu-profiler] Fix script name when recording inlining info

  Use the script name from the shared function info to create an
  inline entry. Otherwise functions are attributed to the wrong file
  in the CpuProfileNode.

  See googleapis/cloud-profiler-nodejs#89

  Bug: v8:7203, v8:7241
  Change-Id: I8ea31943741770e6611275a9c93375922b934547
  Reviewed-on: https://chromium-review.googlesource.com/848093
  Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
  Commit-Queue: Franziska Hinkelmann <franzih@chromium.org>
  Cr-Commit-Position: refs/heads/master@{#50339}

Refs: v8/v8@76c3ac5
PR-URL: #18298
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 21, 2018
Merged
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
@ofrobots @MylesBorins
deps: V8: backport 76c3ac5 from upstream
2a2f6be 
This fixes a bug in the CPU profiler where some ticks were attributed
to the wrong file.

Original commit message:
  [cpu-profiler] Fix script name when recording inlining info

  Use the script name from the shared function info to create an
  inline entry. Otherwise functions are attributed to the wrong file
  in the CpuProfileNode.

  See googleapis/cloud-profiler-nodejs#89

  Bug: v8:7203, v8:7241
  Change-Id: I8ea31943741770e6611275a9c93375922b934547
  Reviewed-on: https://chromium-review.googlesource.com/848093
  Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
  Commit-Queue: Franziska Hinkelmann <franzih@chromium.org>
  Cr-Commit-Position: refs/heads/master@{#50339}

Refs: v8/v8@76c3ac5
PR-URL: #18298
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
@ofrobots @MylesBorins
deps: V8: backport 76c3ac5 from upstream
2d98b58 
This fixes a bug in the CPU profiler where some ticks were attributed
to the wrong file.

Original commit message:
  [cpu-profiler] Fix script name when recording inlining info

  Use the script name from the shared function info to create an
  inline entry. Otherwise functions are attributed to the wrong file
  in the CpuProfileNode.

  See googleapis/cloud-profiler-nodejs#89

  Bug: v8:7203, v8:7241
  Change-Id: I8ea31943741770e6611275a9c93375922b934547
  Reviewed-on: https://chromium-review.googlesource.com/848093
  Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
  Commit-Queue: Franziska Hinkelmann <franzih@chromium.org>
  Cr-Commit-Position: refs/heads/master@{#50339}

Refs: v8/v8@76c3ac5
PR-URL: #18298
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins
Copy link
Contributor

@ofrobots the profiler file doesn't exist on 6.x

MylesBorins pushed a commit that referenced this pull request Apr 13, 2018
@ofrobots @MylesBorins
deps: V8: backport 76c3ac5 from upstream
ab00559 
This fixes a bug in the CPU profiler where some ticks were attributed
to the wrong file.

Original commit message:
  [cpu-profiler] Fix script name when recording inlining info

  Use the script name from the shared function info to create an
  inline entry. Otherwise functions are attributed to the wrong file
  in the CpuProfileNode.

  See googleapis/cloud-profiler-nodejs#89

  Bug: v8:7203, v8:7241
  Change-Id: I8ea31943741770e6611275a9c93375922b934547
  Reviewed-on: https://chromium-review.googlesource.com/848093
  Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
  Commit-Queue: Franziska Hinkelmann <franzih@chromium.org>
  Cr-Commit-Position: refs/heads/master@{#50339}

Refs: v8/v8@76c3ac5
PR-URL: #18298
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
ofrobots added a commit to ofrobots/node that referenced this pull request Apr 19, 2018
@ofrobots
deps: V8: backport 76c3ac5 from upstream
d7972b8 
This is a v6.x backport of 00687fb.

Ref: nodejs#18298
Ref: v8/v8@76c3ac5
@ofrobots ofrobots mentioned this pull request Apr 19, 2018
Closed
2 tasks
@ofrobots
Copy link
Contributor Author

I've done a manual backport to 6.x here: #20156

@MylesBorins MylesBorins mentioned this pull request May 2, 2018
Merged
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
@ofrobots @MayaLekova
deps: V8: backport 76c3ac5 from upstream
d79faca 
This fixes a bug in the CPU profiler where some ticks were attributed
to the wrong file.

Original commit message:
  [cpu-profiler] Fix script name when recording inlining info

  Use the script name from the shared function info to create an
  inline entry. Otherwise functions are attributed to the wrong file
  in the CpuProfileNode.

  See googleapis/cloud-profiler-nodejs#89

  Bug: v8:7203, v8:7241
  Change-Id: I8ea31943741770e6611275a9c93375922b934547
  Reviewed-on: https://chromium-review.googlesource.com/848093
  Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
  Commit-Queue: Franziska Hinkelmann <franzih@chromium.org>
  Cr-Commit-Position: refs/heads/master@{nodejs#50339}

Refs: v8/v8@76c3ac5
PR-URL: nodejs#18298
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@targos targos targos approved these changes

@cjihrig cjihrig cjihrig approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. 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
@ofrobots @MylesBorins @bnoordhuis @targos @cjihrig @nodejs-github-bot