Bug 45036 - [v8] bypass caches when query memory usage from post GC and in crash handler.
Summary: [v8] bypass caches when query memory usage from post GC and in crash handler.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: anton muhin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-01 07:48 PDT by anton muhin
Modified: 2010-09-10 08:33 PDT (History)
3 users (show)

See Also:

Attachments
Patch (6.29 KB, patch)
2010-09-01 08:03 PDT, anton muhin 		no flags Details | Formatted Diff | Diff
Patch (3.55 KB, patch)
2010-09-03 12:15 PDT, anton muhin 		no flags Details | Formatted Diff | Diff
Patch (2.41 KB, patch)
2010-09-08 09:35 PDT, anton muhin 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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