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:

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>