Bug 96756 - Web Inspector: automatically detect if class has reportMemoryUsage method
Summary: Web Inspector: automatically detect if class has reportMemoryUsage method
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Yury Semikhatsky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-14 04:55 PDT by Yury Semikhatsky
Modified: 2012-09-17 02:44 PDT (History)
17 users (show)

See Also:


Attachments
Patch to run on test bots, not for revew (2.99 KB, patch)
2012-09-14 05:29 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Patch (7.16 KB, patch)
2012-09-14 09:36 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Patch (6.77 KB, patch)
2012-09-14 12:00 PDT, Ilya Tikhonovsky
no flags Details | Formatted Diff | Diff
ups. Another patch for try bots with static keyword. (6.79 KB, patch)
2012-09-14 12:47 PDT, Ilya Tikhonovsky
loislo: review-
buildbot: commit-queue-
Details | Formatted Diff | Diff
The patch for windows try bot. I can't reproduce the problem locally. (7.06 KB, patch)
2012-09-15 04:41 PDT, Ilya Tikhonovsky
no flags Details | Formatted Diff | Diff
The patch for windows try bot. I can't reproduce the problem locally. Unused parameter was removed. (7.06 KB, patch)
2012-09-15 04:58 PDT, Ilya Tikhonovsky
no flags Details | Formatted Diff | Diff
comment addressed (8.80 KB, patch)
2012-09-17 00:32 PDT, Ilya Tikhonovsky
apavlov: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Semikhatsky 2012-09-14 04:55:53 PDT
Classes that support memory usage reporting should provide
void reportMemoryUsage(MemoryObjectInfo*) const;
method. If such method is present we should invoke it to collect
memory usage data, otherwise we should just count sizeof the class.
The method detection can be automated at compile time using using
template trick called SFINAE(http://en.wikipedia.org/wiki/Substitution_failure_is_not_an_error).
Comment 1 Yury Semikhatsky 2012-09-14 05:29:43 PDT
Created attachment 164114 [details]
Patch to run on test bots, not for revew
Comment 2 WebKit Review Bot 2012-09-14 05:32:07 PDT
Attachment 164114 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/wtf/MemoryInstrumentation.h']" exit_code: 1
Source/WTF/wtf/MemoryInstrumentation.h:54:  The parameter name "object" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WTF/wtf/MemoryInstrumentation.h:54:  The parameter name "memoryObjectInfo" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 1 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Yury Semikhatsky 2012-09-14 09:36:11 PDT
Created attachment 164170 [details]
Patch
Comment 4 Build Bot 2012-09-14 10:05:11 PDT
Comment on attachment 164170 [details]
Patch

Attachment 164170 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13859240
Comment 5 Ilya Tikhonovsky 2012-09-14 12:00:50 PDT
Created attachment 164202 [details]
Patch
Comment 6 Build Bot 2012-09-14 12:13:32 PDT
Comment on attachment 164202 [details]
Patch

Attachment 164202 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13854532
Comment 7 WebKit Review Bot 2012-09-14 12:20:17 PDT
Comment on attachment 164202 [details]
Patch

Attachment 164202 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13850644
Comment 8 Early Warning System Bot 2012-09-14 12:38:23 PDT
Comment on attachment 164202 [details]
Patch

Attachment 164202 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13844784
Comment 9 Gyuyoung Kim 2012-09-14 12:40:58 PDT
Comment on attachment 164202 [details]
Patch

Attachment 164202 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13858310
Comment 10 Early Warning System Bot 2012-09-14 12:43:46 PDT
Comment on attachment 164202 [details]
Patch

Attachment 164202 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13854540
Comment 11 Ilya Tikhonovsky 2012-09-14 12:47:44 PDT
Created attachment 164209 [details]
ups. Another patch for try bots with static keyword.
Comment 12 Build Bot 2012-09-14 13:13:16 PDT
Comment on attachment 164209 [details]
ups. Another patch for try bots with static keyword.

Attachment 164209 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13845713
Comment 13 Ilya Tikhonovsky 2012-09-15 04:41:55 PDT
Created attachment 164281 [details]
The patch for windows try bot. I can't reproduce the problem locally.
Comment 14 Ilya Tikhonovsky 2012-09-15 04:58:23 PDT
Created attachment 164282 [details]
The patch for windows try bot. I can't reproduce the problem locally. Unused parameter was removed.
Comment 15 Yury Semikhatsky 2012-09-16 23:22:40 PDT
Comment on attachment 164282 [details]
The patch for windows try bot. I can't reproduce the problem locally. Unused parameter was removed.

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

> Source/WTF/wtf/MemoryInstrumentation.h:95
> +        countNotInstrumentedObject(object, memoryObjectInfo);

MemoryObjectInfo could be moved above MemoryInstrumentation in this header, then this method could be inlined.
Comment 16 Ilya Tikhonovsky 2012-09-17 00:32:44 PDT
Created attachment 164350 [details]
comment addressed
Comment 17 Alexander Pavlov (apavlov) 2012-09-17 01:15:41 PDT
Comment on attachment 164350 [details]
comment addressed

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

> Source/WTF/ChangeLog:8
> +        Implemeted automatic selector of the memory reporting method. If

typo: Implemented

> Source/WebKit/chromium/tests/MemoryInstrumentationTest.cpp:294
> +TEST(MemoryInstrumentationTest, detectReportMemoryUsageMethod)

This test should either test both memory usage reporting methods or have a different name that implies only the capability of addMember() to reportMemoryUsage() (but not reportObjectInfo()) is tested.
Comment 18 Ilya Tikhonovsky 2012-09-17 02:44:04 PDT
Committed r128732: <http://trac.webkit.org/changeset/128732>