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

Issue 994503005: Oilpan: ENABLE(GC_PROFILING): Don't emit free list entries for LargeObject heap (Closed)

Created:
5 years, 9 months ago by kouhei (in TOK)
Modified:
5 years, 9 months ago
Reviewers:
keishi, oilpan-reviews
CC:
blink-reviews, oilpan-reviews, kouhei+heap_chromium.org, Mads Ager (chromium)
Target Ref:
refs/remotes/origin/master
Project:
blink
Visibility:
Public.

Description

Oilpan: ENABLE(GC_PROFILING): Don't emit free list entries for LargeObject heap Before this CL, the tracing.json output by the profiler couldn't be parsed correctly by dino.js visualizer, as it contained invalid entries from LargeObject heap. This CL fixes the issue by disabling dump of free list entries for LargeObject heap. BUG=454273

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -1 line) Patch
M Source/platform/heap/ThreadState.cpp View 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 7 (1 generated)
kouhei (in TOK)
5 years, 9 months ago (2015-03-10 02:15:25 UTC) #2
haraken
This fix is ok, but can we fix LargeObjectHeap::snapshotFreeList so that it doesn't output invalid ...
5 years, 9 months ago (2015-03-10 02:18:31 UTC) #3
kouhei (in TOK)
haraken: According to keishi and tkent, LargeObject doesn't have a free list so this patch ...
5 years, 9 months ago (2015-03-10 09:09:06 UTC) #4
haraken
On 2015/03/10 09:09:06, kouhei wrote: > haraken: According to keishi and tkent, LargeObject doesn't have ...
5 years, 9 months ago (2015-03-10 11:05:48 UTC) #5
keishi
On 2015/03/10 11:05:48, haraken wrote: > On 2015/03/10 09:09:06, kouhei wrote: > > haraken: According ...
5 years, 9 months ago (2015-03-11 02:25:24 UTC) #6
kouhei (in TOK)
5 years, 9 months ago (2015-03-11 05:51:42 UTC) #7
On 2015/03/11 02:25:24, keishi wrote:
> On 2015/03/10 11:05:48, haraken wrote:
> > On 2015/03/10 09:09:06, kouhei wrote:
> > > haraken: According to keishi and tkent, LargeObject doesn't have a free
list
> > so
> > > this patch should be correct? The other stats for the LargeObject heap is
> > > collected from ThreadState::snapshot().
> > 
> > You're right. However, snapshotFreeList is a virtual method of BaseHeap, so
I
> > think it should be allowed to call snapshotFreeList for LargeObjectHeap
> > (LargeObjectHeap::snapshotFreeList is already implemented so that it does
> > nothing). I guess the profiler should be fixed so that it doesn't crash when
> it
> > encounters an empty entry in the json.
> 
> Done. Let me know if it doesn't work.

Thanks! closing this as the patch is no longer needed.

Powered by Google App Engine
This is Rietveld 408576698