Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(287)

Issue 12212157: Get a filename of dumped heap profile in a V8 extension HeapProfilerDump(). (Closed)

Created:
7 years, 10 months ago by Dai Mikurube (NOT FULLTIME)
Modified:
7 years, 10 months ago
CC:
chromium-reviews, jam, joi+watch-content_chromium.org, Aaron Boodman, darin-cc_chromium.org, dmikurube+memory_chromium.org, chromium-apps-reviews_chromium.org, dennis_jeffrey
Visibility:
Public.

Description

Get a filename of dumped heap profile in a V8 extension HeapProfilerDump(). BUG=chromium-os:38652 TBR=darin@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=182659

Patch Set 1 #

Total comments: 1

Patch Set 2 : updated the comment #

Total comments: 2

Patch Set 3 : addressed the comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+38 lines, -13 lines) Patch
M content/renderer/memory_benchmarking_extension.cc View 2 chunks +6 lines, -2 lines 0 comments Download
M third_party/tcmalloc/chromium/src/gperftools/heap-profiler.h View 1 1 chunk +9 lines, -0 lines 0 comments Download
M third_party/tcmalloc/chromium/src/heap-profiler.cc View 1 2 5 chunks +23 lines, -11 lines 0 comments Download

Messages

Total messages: 20 (0 generated)
Dai Mikurube (NOT FULLTIME)
Hi Jim and Nat, It's * adding a new TCMalloc heap-profiler function HeapProfilerDumpWithFilename() * changing ...
7 years, 10 months ago (2013-02-13 11:30:51 UTC) #1
Dai Mikurube (NOT FULLTIME)
(fyi, another reason why I didn't choose giving a filename is that the heap profiler ...
7 years, 10 months ago (2013-02-13 11:34:08 UTC) #2
Dai Mikurube (NOT FULLTIME)
Added a note. https://codereview.chromium.org/12212157/diff/1/third_party/tcmalloc/chromium/src/gperftools/heap-profiler.h File third_party/tcmalloc/chromium/src/gperftools/heap-profiler.h (right): https://codereview.chromium.org/12212157/diff/1/third_party/tcmalloc/chromium/src/gperftools/heap-profiler.h#newcode99 third_party/tcmalloc/chromium/src/gperftools/heap-profiler.h:99: int filename_buffer_length); FYI, |dumped_filename_buffer| is output ...
7 years, 10 months ago (2013-02-13 16:04:45 UTC) #3
Dai Mikurube (NOT FULLTIME)
Gentle ping? Updated the comment as written in https://codereview.chromium.org/12212157/diff/1/third_party/tcmalloc/chromium/src/gperftools/heap-profiler.h#newcode99.
7 years, 10 months ago (2013-02-14 07:27:35 UTC) #4
nduca
extension portion lgtm. I know jar got nailed with a dozen different p0 reviews today, ...
7 years, 10 months ago (2013-02-14 07:31:36 UTC) #5
Dai Mikurube (NOT FULLTIME)
On 2013/02/14 07:31:36, nduca wrote: > extension portion lgtm. > > I know jar got ...
7 years, 10 months ago (2013-02-14 08:43:26 UTC) #6
jar (doing other things)
Small nit below. LGTM https://codereview.chromium.org/12212157/diff/4001/third_party/tcmalloc/chromium/src/heap-profiler.cc File third_party/tcmalloc/chromium/src/heap-profiler.cc (right): https://codereview.chromium.org/12212157/diff/4001/third_party/tcmalloc/chromium/src/heap-profiler.cc#newcode277 third_party/tcmalloc/chromium/src/heap-profiler.cc:277: int filename_buffer_length) { nit: int-->size_t
7 years, 10 months ago (2013-02-14 23:30:09 UTC) #7
Dai Mikurube (NOT FULLTIME)
Thanks, Jim! I'll be checking the "Commit" box. https://codereview.chromium.org/12212157/diff/4001/third_party/tcmalloc/chromium/src/heap-profiler.cc File third_party/tcmalloc/chromium/src/heap-profiler.cc (right): https://codereview.chromium.org/12212157/diff/4001/third_party/tcmalloc/chromium/src/heap-profiler.cc#newcode277 third_party/tcmalloc/chromium/src/heap-profiler.cc:277: int ...
7 years, 10 months ago (2013-02-15 03:30:42 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dmikurube@chromium.org/12212157/8001
7 years, 10 months ago (2013-02-15 03:34:20 UTC) #9
commit-bot: I haz the power
Presubmit check for 12212157-8001 failed and returned exit status 1. INFO:root:Found 3 file(s). Running presubmit ...
7 years, 10 months ago (2013-02-15 03:34:27 UTC) #10
Dai Mikurube (NOT FULLTIME)
+piman I forgot the OWNERS again. ;) piman@, could you take a look? nduca@ has ...
7 years, 10 months ago (2013-02-15 05:08:59 UTC) #11
piman
lgtm
7 years, 10 months ago (2013-02-15 05:26:59 UTC) #12
Dai Mikurube (NOT FULLTIME)
On 2013/02/15 05:26:59, piman wrote: > lgtm Thanks!
7 years, 10 months ago (2013-02-15 05:40:47 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dmikurube@chromium.org/12212157/8001
7 years, 10 months ago (2013-02-15 05:41:42 UTC) #14
commit-bot: I haz the power
Presubmit check for 12212157-8001 failed and returned exit status 1. INFO:root:Found 3 file(s). Running presubmit ...
7 years, 10 months ago (2013-02-15 05:41:45 UTC) #15
Dai Mikurube (NOT FULLTIME)
+Darin, Ugh, third_party/ recently has OWNERS. Darin, could you take a look? It's for the ...
7 years, 10 months ago (2013-02-15 05:52:53 UTC) #16
Dai Mikurube (NOT FULLTIME)
Sorry, I missed the announcement of thitd_party's OWNERS. I added darin to TBR. I'll be ...
7 years, 10 months ago (2013-02-15 05:59:22 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dmikurube@chromium.org/12212157/8001
7 years, 10 months ago (2013-02-15 06:00:05 UTC) #18
darin (slow to review)
LGTM Yes, adding an OWNERS file for tcmalloc seems like a good idea.
7 years, 10 months ago (2013-02-15 06:01:34 UTC) #19
commit-bot: I haz the power
7 years, 10 months ago (2013-02-15 10:12:31 UTC) #20
Message was sent while issue was closed.
Change committed as 182659

Powered by Google App Engine
This is Rietveld 408576698