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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 145353003: Allow arbitrary names for weak references in heap snapshots. (Closed)

Created:
6 years, 11 months ago by alph
Modified:
6 years, 11 months ago
Reviewers:
tfarina, ulan, yurys, loislo
CC:
v8-dev
Visibility:
Public.

Description

Allow arbitrary names for weak references in heap snapshots. LOG=N R=ulan@chromium.org, yurys@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=18838

Patch Set 1 #

Total comments: 1

Patch Set 2 : Adding more checks. #

Patch Set 3 : Rebaseline + updated weak in couple other places. #

Patch Set 4 : Reupload #

Patch Set 5 : Reupload #

Patch Set 6 : Reupload #

Patch Set 7 : Reupload #

Patch Set 8 : Reupload #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+53 lines, -35 lines) Patch
src/api.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M src/heap-snapshot-generator.h View 1 2 2 chunks +4 lines, -3 lines 0 comments Download
src/heap-snapshot-generator.cc View 1 2 11 chunks +47 lines, -30 lines 2 comments Download
M test/cctest/test-heap-profiler.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 9 (0 generated)
alph
ptal
6 years, 11 months ago (2014-01-24 13:54:46 UTC) #1
yurys
lgtm https://codereview.chromium.org/145353003/diff/1/src/heap-snapshot-generator.cc File src/heap-snapshot-generator.cc (left): https://codereview.chromium.org/145353003/diff/1/src/heap-snapshot-generator.cc#oldcode1120 src/heap-snapshot-generator.cc:1120: SetWeakReference(js_fun, entry, i, *HeapObject::RawField(js_fun, i), i); Please add ...
6 years, 11 months ago (2014-01-24 14:02:55 UTC) #2
alph
Ulan, could you please make owner's review.
6 years, 11 months ago (2014-01-24 15:02:30 UTC) #3
ulan
lgtm
6 years, 11 months ago (2014-01-24 15:07:58 UTC) #4
alph
Committed patchset #8 manually as r18838 (presubmit successful).
6 years, 11 months ago (2014-01-24 17:18:43 UTC) #5
tfarina
https://codereview.chromium.org/145353003/diff/240001/src/heap-snapshot-generator.cc File src/heap-snapshot-generator.cc (right): https://codereview.chromium.org/145353003/diff/240001/src/heap-snapshot-generator.cc#newcode1217 src/heap-snapshot-generator.cc:1217: EXTRACT_CONTEXT_FIELD(OPTIMIZED_FUNCTIONS_LIST, , optimized_functions_list); looks like this broke the clang ...
6 years, 11 months ago (2014-01-27 01:05:28 UTC) #6
tfarina
https://codereview.chromium.org/145353003/diff/240001/src/heap-snapshot-generator.cc File src/heap-snapshot-generator.cc (right): https://codereview.chromium.org/145353003/diff/240001/src/heap-snapshot-generator.cc#newcode1217 src/heap-snapshot-generator.cc:1217: EXTRACT_CONTEXT_FIELD(OPTIMIZED_FUNCTIONS_LIST, , optimized_functions_list); looks like this broke the clang ...
6 years, 11 months ago (2014-01-27 01:05:28 UTC) #7
alph
https://codereview.chromium.org/145353003/diff/240001/src/heap-snapshot-generator.cc File src/heap-snapshot-generator.cc (right): https://codereview.chromium.org/145353003/diff/240001/src/heap-snapshot-generator.cc#newcode1217 src/heap-snapshot-generator.cc:1217: EXTRACT_CONTEXT_FIELD(OPTIMIZED_FUNCTIONS_LIST, , optimized_functions_list); On 2014/01/27 01:05:29, tfarina wrote: > ...
6 years, 11 months ago (2014-01-27 09:09:20 UTC) #8
tfarina
6 years, 11 months ago (2014-01-27 15:50:28 UTC) #9
Message was sent while issue was closed. 
On 2014/01/27 09:09:20, alph wrote:
>
https://codereview.chromium.org/145353003/diff/240001/src/heap-snapshot-gener...
> File src/heap-snapshot-generator.cc (right):
> 
>
https://codereview.chromium.org/145353003/diff/240001/src/heap-snapshot-gener...
> src/heap-snapshot-generator.cc:1217:
> EXTRACT_CONTEXT_FIELD(OPTIMIZED_FUNCTIONS_LIST, , optimized_functions_list);
> On 2014/01/27 01:05:29, tfarina wrote:
> > looks like this broke the clang build on linux x64:
> > 
> > $ ninja -C out/Debug/ All
> > ....
> > ../../src/heap-snapshot-generator.cc:1217:53: error: empty macro arguments
are
> a
> > C99 feature [-Werror,-Wc99-extensions]
> >     EXTRACT_CONTEXT_FIELD(OPTIMIZED_FUNCTIONS_LIST, ,
> optimized_functions_list);
> >                                                     ^
> > ../../src/heap-snapshot-generator.cc:1218:48: error: empty macro arguments
are
> a
> > C99 feature [-Werror,-Wc99-extensions]
> >     EXTRACT_CONTEXT_FIELD(OPTIMIZED_CODE_LIST, , optimized_code_list);
> >                                                ^
> > ../../src/heap-snapshot-generator.cc:1219:50: error: empty macro arguments
are
> a
> > C99 feature [-Werror,-Wc99-extensions]
> >     EXTRACT_CONTEXT_FIELD(DEOPTIMIZED_CODE_LIST, , deoptimized_code_list);
> >                                                  ^
> > ../../src/heap-snapshot-generator.cc:1220:46: error: empty macro arguments
are
> a
> > C99 feature [-Werror,-Wc99-extensions]
> >     EXTRACT_CONTEXT_FIELD(NEXT_CONTEXT_LINK, , next_context_link);
> >                                              ^
> > 4 errors generated.
> > 
> > $ clang++ --version
> > clang version 3.5 (trunk 198389)
> > Target: x86_64-unknown-linux-gnu
> > Thread model: posix
> 
> Strange, I also use clang and it compiles just fine for me:
> $ ninja -j 1000 -C out/Debug chrome 
> ninja: Entering directory `out/Debug'
> [13749/13749] LINK chrome
Did your change rolled into chrome already (i.e., by the time you tested this)?
I'm building in v8 repo not in the chromium repo. I'd expect you too build in
the v8 repo.

> $ clang++ --version
> clang version 3.5 (trunk 198389)
> Target: x86_64-unknown-linux-gnu
> Thread model: posix
> 
> Nevertheless, I'll add a dummy argument.
Thanks!

Powered by Google App Engine
This is Rietveld 408576698