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
anton muhin
2010-09-01 07:48:12 PDT
Created attachment 66212 [details]
Patch
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. (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. Created attachment 66531 [details]
Patch
Adam, may you have a look? Now I should really start with extending Chromium's API. 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 on attachment 66531 [details]
Patch
Thanks a lot for review, Adam.
Comment on attachment 66531 [details] Patch Clearing flags on attachment: 66531 Committed r66818: <http://trac.webkit.org/changeset/66818> All reviewed patches have been landed. Closing bug. Created attachment 66906 [details]
Patch
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. Adam, may you have a look, please? Comment on attachment 66906 [details] Patch Clearing flags on attachment: 66906 Committed r67111: <http://trac.webkit.org/changeset/67111> All reviewed patches have been landed. Closing bug. Thanks a lot for review, Adam |