Bug 45036

Summary: [v8] bypass caches when query memory usage from post GC and in crash handler.
Product: WebKit Reporter: anton muhin <antonm>
Component: WebCore Misc.Assignee: anton muhin <antonm>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, thakis
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description anton muhin 2010-09-01 07:48:12 PDT 
[v8] bypass caches when query memory usage from post GC and in crash handler.
Comment 1 anton muhin 2010-09-01 08:03:22 PDT 
Created attachment 66212 [details]
Patch
Comment 2 Adam Barth 2010-09-01 11:35:04 PDT 
Comment on attachment 66212 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=66212&action=prettypatch

> WebCore/bindings/v8/V8GCController.cpp:400
> -int getMemoryUsageInMB()
> +int getMemoryUsageInMB(bool noCache)
Instead of using a bool, please use an enum so we can understand what passing "true" means at the callsites.
Comment 3 anton muhin 2010-09-02 08:24:58 PDT 
(In reply to comment #2)
> (From update of attachment 66212 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=66212&action=prettypatch
> 
> > WebCore/bindings/v8/V8GCController.cpp:400
> > -int getMemoryUsageInMB()
> > +int getMemoryUsageInMB(bool noCache)
> Instead of using a bool, please use an enum so we can understand what passing "true" means at the callsites.

Adam, Chromium's counterpart to this change has been backed out, so, please, let put on hold this review.  I'll address your comment and request for another round when I am done with Chromium side.

Sorry for troubles.
Comment 4 anton muhin 2010-09-03 12:15:05 PDT 
Created attachment 66531 [details]
Patch
Comment 5 anton muhin 2010-09-03 12:16:02 PDT 
Adam, may you have a look?

Now I should really start with extending Chromium's API.
Comment 6 Adam Barth 2010-09-05 23:37:56 PDT 
Comment on attachment 66531 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=66531&action=prettypatch

> WebCore/platform/chromium/ChromiumBridge.h:194
> +        // Same as above, but always returns actual value, without any caches.
> +        static int actualMemoryUsageMB();
Much nicer.
Comment 7 anton muhin 2010-09-06 01:06:37 PDT 
Comment on attachment 66531 [details]
Patch

Thanks a lot for review, Adam.
Comment 8 WebKit Commit Bot 2010-09-06 01:31:57 PDT 
Comment on attachment 66531 [details]
Patch

Clearing flags on attachment: 66531

Committed r66818: <http://trac.webkit.org/changeset/66818>
Comment 9 WebKit Commit Bot 2010-09-06 01:32:01 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 10 anton muhin 2010-09-08 09:35:09 PDT 
Created attachment 66906 [details]
Patch
Comment 11 anton muhin 2010-09-08 09:36:48 PDT 
Reopend as that was two sided patch:

1) introduce API in the bridge;
2) commit implementation of API into Chromium proper;
3) actually use the API.

The latest patch corresponds to phase 3.
Comment 12 anton muhin 2010-09-09 10:35:36 PDT 
Adam, may you have a look, please?
Comment 13 WebKit Commit Bot 2010-09-09 13:50:13 PDT 
Comment on attachment 66906 [details]
Patch

Clearing flags on attachment: 66906

Committed r67111: <http://trac.webkit.org/changeset/67111>
Comment 14 WebKit Commit Bot 2010-09-09 13:50:17 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 15 anton muhin 2010-09-10 08:33:53 PDT 
Thanks a lot for review, Adam