Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2519)

Issue 153750043: code review 153750043: cmd/pprof: add Go implementation (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 2 months ago by rsc
Modified:
10 years, 2 months ago
Reviewers:
r, gobot
CC:
r, bradfitz, brainman, dave_cheney.net, golang-codereviews
Visibility:
Public.

Description

cmd/pprof: add Go implementation Update issue 8798 This is a new implementation of pprof, written in Go instead of in Perl. It was written primarily by Raul Silvera and is in use for profiling programs of all languages inside Google. The internal structure is a bit package-heavy, but it matches the copy used inside Google, and since it is in an internal directory, we can make changes to it later if we need to. The only "new" file here is src/cmd/pprof/pprof.go, which stitches together the Google pprof and the Go command libraries for object file access. I am explicitly NOT interested in style or review comments on the rest of the files (that is, src/cmd/pprof/internal/...). Those are intended to stay as close to the Google copies as possible, like we did with the pprof Perl script. Still to do: - Basic tests. - Real command documentation. - Hook up disassemblers.

Patch Set 1 #

Patch Set 2 : diff -r cd24f5ff3db869a41efbf3a83e9e3f55b61460b8 https://code.google.com/p/go/ #

Patch Set 3 : diff -r cd24f5ff3db869a41efbf3a83e9e3f55b61460b8 https://code.google.com/p/go/ #

Total comments: 21

Patch Set 4 : diff -r 6e19c3e6b95c0efb627a5ab98187957c325b563a https://code.google.com/p/go/ #

Total comments: 1

Patch Set 5 : diff -r 6e19c3e6b95c0efb627a5ab98187957c325b563a https://code.google.com/p/go/ #

Patch Set 6 : diff -r 2e467bc60e64def06419194f48fe0d6c8b56765d https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7728 lines, -0 lines) Patch
A src/cmd/pprof/internal/commands/commands.go View 1 2 1 chunk +197 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/driver/driver.go View 1 2 1 chunk +1036 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/driver/interactive.go View 1 2 1 chunk +492 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/fetch/fetch.go View 1 2 1 chunk +82 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/plugin/plugin.go View 1 2 1 chunk +213 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/profile/encode.go View 1 2 1 chunk +470 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/profile/filter.go View 1 2 1 chunk +157 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/profile/legacy_profile.go View 1 2 1 chunk +1250 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/profile/profile.go View 1 2 1 chunk +567 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/profile/proto.go View 1 2 1 chunk +298 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/profile/prune.go View 1 2 1 chunk +97 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/report/report.go View 1 2 1 chunk +1718 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/report/source.go View 1 2 1 chunk +450 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/report/source_html.go View 1 2 1 chunk +77 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/svg/svg.go View 1 2 1 chunk +75 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/symbolizer/symbolizer.go View 1 2 1 chunk +191 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/symbolz/symbolz.go View 1 2 1 chunk +111 lines, -0 lines 0 comments Download
A src/cmd/pprof/internal/tempfile/tempfile.go View 1 2 1 chunk +45 lines, -0 lines 0 comments Download
A src/cmd/pprof/pprof.go View 1 2 3 4 1 chunk +202 lines, -0 lines 0 comments Download

Messages

Total messages: 15
rsc
Hello r (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go/
10 years, 2 months ago (2014-09-29 16:18:32 UTC) #1
bradfitz
https://codereview.appspot.com/153750043/diff/40001/src/cmd/pprof/pprof.go File src/cmd/pprof/pprof.go (right): https://codereview.appspot.com/153750043/diff/40001/src/cmd/pprof/pprof.go#newcode5 src/cmd/pprof/pprof.go:5: package main no package comment https://codereview.appspot.com/153750043/diff/40001/src/cmd/pprof/pprof.go#newcode25 src/cmd/pprof/pprof.go:25: if err ...
10 years, 2 months ago (2014-09-29 16:28:31 UTC) #2
r
the author should be in CONTRIBUTORS
10 years, 2 months ago (2014-09-29 17:35:41 UTC) #3
rsc
On Mon, Sep 29, 2014 at 1:35 PM, <r@golang.org> wrote: > the author should be ...
10 years, 2 months ago (2014-09-29 18:02:40 UTC) #4
brainman
This even runs on windows! Thank you Russ. Alex
10 years, 2 months ago (2014-09-29 23:19:38 UTC) #5
r
https://codereview.appspot.com/153750043/diff/40001/src/cmd/pprof/pprof.go File src/cmd/pprof/pprof.go (right): https://codereview.appspot.com/153750043/diff/40001/src/cmd/pprof/pprof.go#newcode30 src/cmd/pprof/pprof.go:30: // symbolize attempts to symbolize profile p. First uses ...
10 years, 2 months ago (2014-09-29 23:44:28 UTC) #6
dave_cheney.net
Does this code need to be in the main repository ? Could it be moved ...
10 years, 2 months ago (2014-09-30 00:21:39 UTC) #7
rsc
On Mon, Sep 29, 2014 at 8:21 PM, Dave Cheney <dave@cheney.net> wrote: > Does this ...
10 years, 2 months ago (2014-09-30 00:52:34 UTC) #8
dave_cheney.net
Fair enough. Sorry for the noise. On Tue, Sep 30, 2014 at 10:52 AM, Russ ...
10 years, 2 months ago (2014-09-30 00:54:21 UTC) #9
rsc
PTAL https://codereview.appspot.com/153750043/diff/40001/src/cmd/pprof/pprof.go File src/cmd/pprof/pprof.go (right): https://codereview.appspot.com/153750043/diff/40001/src/cmd/pprof/pprof.go#newcode5 src/cmd/pprof/pprof.go:5: package main On 2014/09/29 16:28:31, bradfitz wrote: > ...
10 years, 2 months ago (2014-09-30 16:43:24 UTC) #10
bradfitz
https://codereview.appspot.com/153750043/diff/40001/src/cmd/pprof/pprof.go File src/cmd/pprof/pprof.go (right): https://codereview.appspot.com/153750043/diff/40001/src/cmd/pprof/pprof.go#newcode25 src/cmd/pprof/pprof.go:25: if err := driver.PProf(flags{}, fetch.Fetcher, symbolize, new(objTool), plugin.StandardUI(), nil); ...
10 years, 2 months ago (2014-09-30 16:47:46 UTC) #11
rsc
https://codereview.appspot.com/153750043/diff/40001/src/cmd/pprof/pprof.go File src/cmd/pprof/pprof.go (right): https://codereview.appspot.com/153750043/diff/40001/src/cmd/pprof/pprof.go#newcode25 src/cmd/pprof/pprof.go:25: if err := driver.PProf(flags{}, fetch.Fetcher, symbolize, new(objTool), plugin.StandardUI(), nil); ...
10 years, 2 months ago (2014-09-30 17:29:22 UTC) #12
r
LGTM with behind-the-scenes grumbling. https://codereview.appspot.com/153750043/diff/60001/src/cmd/pprof/pprof.go File src/cmd/pprof/pprof.go (right): https://codereview.appspot.com/153750043/diff/60001/src/cmd/pprof/pprof.go#newcode45 src/cmd/pprof/pprof.go:45: ui.PrintErr("expecting -symbolize=[local|remote|none][:force]") i know you ...
10 years, 2 months ago (2014-09-30 17:29:48 UTC) #13
rsc
*** Submitted as https://code.google.com/p/go/source/detail?r=c1a3f7652520 *** cmd/pprof: add Go implementation Update issue 8798 This is a ...
10 years, 2 months ago (2014-09-30 17:42:00 UTC) #14
gobot
10 years, 2 months ago (2014-09-30 17:51:45 UTC) #15
Message was sent while issue was closed.
This CL appears to have broken the plan9-amd64-aram builder.
See http://build.golang.org/log/4f60dd6f747dbc30ae8053968760f88318dd9fff
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b