Bug 96756

Summary: Web Inspector: automatically detect if class has reportMemoryUsage method
Product: WebKit Reporter: Yury Semikhatsky <yurys>
Component: Web Inspector (Deprecated)Assignee: Yury Semikhatsky <yurys>
Status: RESOLVED FIXED    
Severity: Normal CC: alph, apavlov, benjamin, bweinstein, dglazkov, gustavo, joepeck, keishi, loislo, pfeldman, philn, pmuellr, rik, timothy, webkit.review.bot, xan.lopez, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch to run on test bots, not for revew
none
Patch
none
Patch
none
ups. Another patch for try bots with static keyword.
loislo: review-, buildbot: commit-queue-
The patch for windows try bot. I can't reproduce the problem locally.
none
The patch for windows try bot. I can't reproduce the problem locally. Unused parameter was removed.
none
comment addressed apavlov: review+

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>