RESOLVED FIXED 45036
[v8] bypass caches when query memory usage from post GC and in crash handler. 
https://bugs.webkit.org/show_bug.cgi?id=45036
Summary [v8] bypass caches when query memory usage from post GC and in crash handler.
anton muhin
Reported 2010-09-01 07:48:12 PDT
[v8] bypass caches when query memory usage from post GC and in crash handler.
Attachments
Patch (6.29 KB, patch)
2010-09-01 08:03 PDT, anton muhin 		no flags
DetailsFormatted DiffDiff
Patch (3.55 KB, patch)
2010-09-03 12:15 PDT, anton muhin 		no flags
DetailsFormatted DiffDiff
Patch (2.41 KB, patch)
2010-09-08 09:35 PDT, anton muhin 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
anton muhin
Comment 1 2010-09-01 08:03:22 PDT
Created attachment 66212 [details] Patch
Adam Barth
Comment 2 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.
anton muhin
Comment 3 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.
anton muhin
Comment 4 2010-09-03 12:15:05 PDT
Created attachment 66531 [details] Patch
anton muhin
Comment 5 2010-09-03 12:16:02 PDT
Adam, may you have a look? Now I should really start with extending Chromium's API.
Adam Barth
Comment 6 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.
anton muhin
Comment 7 2010-09-06 01:06:37 PDT
Comment on attachment 66531 [details] Patch Thanks a lot for review, Adam.
WebKit Commit Bot
Comment 8 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>
WebKit Commit Bot
Comment 9 2010-09-06 01:32:01 PDT
All reviewed patches have been landed. Closing bug.
anton muhin
Comment 10 2010-09-08 09:35:09 PDT
Created attachment 66906 [details] Patch
anton muhin
Comment 11 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.
anton muhin
Comment 12 2010-09-09 10:35:36 PDT
Adam, may you have a look, please?
WebKit Commit Bot
Comment 13 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>
WebKit Commit Bot
Comment 14 2010-09-09 13:50:17 PDT
All reviewed patches have been landed. Closing bug.
anton muhin
Comment 15 2010-09-10 08:33:53 PDT
Thanks a lot for review, Adam
Note You need to log in before you can comment on or make changes to this bug.