Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1776853003:
Add Linux perf profiling wrapper script. (Closed)
|
Created:
4 years, 9 months ago by Stefano Sanfilippo Modified:
4 years, 9 months ago Reviewers:
Michael Achenbach CC:
v8-reviews_googlegroups.com, rmcilroy Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionAdd Linux perf profiling wrapper script.
LOG=N
Committed: https://crrev.com/4cf44f3a655ad612a3ffd7d4c065a383601e926e
Cr-Commit-Position: refs/heads/master@{#34689}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Fix bashisms. #Patch Set 3 : No "benchmark" in run message. #Messages
Total messages: 17 (6 generated)
ssanfilippo@chromium.org changed reviewers: + machenbach@chromium.org
This script wraps a d8 command line around perf, saving a few keystrokes for setting kernel variables and enabling collection of callchains. It is similar to run-llprof.sh, but it differs in that the command line is not geared towards ll_prof.py, namely there's no --llprof, and call graph recording is enabled. PTAL.
https://codereview.chromium.org/1776853003/diff/1/tools/run-perf.sh File tools/run-perf.sh (right): https://codereview.chromium.org/1776853003/diff/1/tools/run-perf.sh#newcode1 tools/run-perf.sh:1: #! /bin/bash bash and not sh because of the == between strings below. Should we fix this bashism?
lgtm (mostly rubber-stamp) I assume this is intended to be run manually not on the bots right? Maybe consider creating a new meaningful subdir in tools as the tools folder itself is a bit bloated.
On 2016/03/10 11:56:01, Michael Achenbach wrote: > Maybe consider creating a new meaningful subdir in tools as the tools folder > itself is a bit bloated. +1. We should clean up perf profiling a bit, let us talk next week.
On 2016/03/10 11:56:01, Michael Achenbach wrote: > I assume this is intended to be run manually not on the bots right? Yes exactly. It's meant as a shortcut to avoid having to type a perf command line every time. I wrote it (mostly copying run-llprof.sh) to be used in conjunction with the other two CLs I have in flight, but there's nothing Ignition specific in it.
On 2016/03/10 11:59:21, Jarin wrote: > +1. We should clean up perf profiling a bit, let us talk next week. Definitely. In the meantime, shall I put this CL on hold? Or land it, and then we'll discuss how to reorganize things?
The CQ bit was checked by ssanfilippo@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1776853003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1776853003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by ssanfilippo@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from machenbach@chromium.org Link to the patchset: https://codereview.chromium.org/1776853003/#ps40001 (title: "No "benchmark" in run message.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1776853003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1776853003/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Add Linux perf profiling wrapper script. LOG=N ========== to ========== Add Linux perf profiling wrapper script. LOG=N Committed: https://crrev.com/4cf44f3a655ad612a3ffd7d4c065a383601e926e Cr-Commit-Position: refs/heads/master@{#34689} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/4cf44f3a655ad612a3ffd7d4c065a383601e926e Cr-Commit-Position: refs/heads/master@{#34689} |
