Created attachment 105570[details]
First patch which just lays the groundwork to fetch performance statistics from WebProcess.
I'll add code to actually fetch the WebCore statistics in future patches.
Attachment 105570[details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/CMakeLists.txt', u'Source/W..." exit_code: 1
Source/WebKit2/UIProcess/WebContext.h:207: The parameter name "statisticsData" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 1 in 24 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 105580[details]
First patch which just lays the groundwork to fetch performance statistics from WebProcess.
Fixed an issue found by style-queue (removed the parameter name "statisticsData" from the declaration of WebContext::didGetWebCoreStatistics()).
Comment on attachment 105580[details]
First patch which just lays the groundwork to fetch performance statistics from WebProcess.
View in context: https://bugs.webkit.org/attachment.cgi?id=105580&action=review> Source/WebKit2/UIProcess/API/C/WKContext.cpp:235
> +void WKContextGetStatistics(WKContextRef contextRef, WKContextGetStatisticsFunction callback, void* context)
In the other places where we have callback functions, we tend to have the context first, then the callback. See WKPageGetContentsAsString
Comment on attachment 105580[details]
First patch which just lays the groundwork to fetch performance statistics from WebProcess.
I’m not sure the statistics result should come back as an object of its own custom class. Another way to do it is to return a dictionary with keys and values for the various statistics or use a struct. A reference counted object to hold the data doesn’t seem significantly better than those other two options.
(In reply to comment #5)
> (From update of attachment 105580[details])
> I’m not sure the statistics result should come back as an object of its own custom class. Another way to do it is to return a dictionary with keys and values for the various statistics or use a struct. A reference counted object to hold the data doesn’t seem significantly better than those other two options.
That's a good idea. I'm going to rework the patch to eliminate the custom APIObject class for packaging the statistics.
Created attachment 105675[details]
First patch which just lays the groundwork to fetch performance statistics from WebProcess.
Patch was modified to return the statistics in a dictionary.
Comment on attachment 105812[details]
Gather JavaScript, FastMalloc, icon, font, and glyph page statistics in WebProcess::getWebCoreStatistics()
View in context: https://bugs.webkit.org/attachment.cgi?id=105812&action=review> Source/WebKit2/UIProcess/WebContext.cpp:807
> + RefPtr<MutableDictionary> result = MutableDictionary::create();
> + HashMap<String, uint32_t>::const_iterator end = map.end();
> + for (HashMap<String, uint32_t>::const_iterator it = map.begin(); it != end; ++it)
> + result->set(it->first, RefPtr<WebUInt64>(WebUInt64::create(it->second)).get());
> +
It seems odd that we are using uint32_t and then making the API uint64_t. I think the internal representation (in the HashMap) should be uint64_t.
> Source/WebKit2/UIProcess/WebContext.cpp:820
> + statistics->set("JavaScriptProtectedObjectTypeCounts", RefPtr<MutableDictionary>(createDictionaryFromHashMap(statisticsData.javaScriptProtectedObjectTypeCounts)).get());
The RefPtr car should no be necessary.
(In reply to comment #9)
> (From update of attachment 105812[details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=105812&action=review
>
> > Source/WebKit2/UIProcess/WebContext.cpp:807
> > + RefPtr<MutableDictionary> result = MutableDictionary::create();
> > + HashMap<String, uint32_t>::const_iterator end = map.end();
> > + for (HashMap<String, uint32_t>::const_iterator it = map.begin(); it != end; ++it)
> > + result->set(it->first, RefPtr<WebUInt64>(WebUInt64::create(it->second)).get());
> > +
>
> It seems odd that we are using uint32_t and then making the API uint64_t. I think the internal representation (in the HashMap) should be uint64_t.
Changed to uint64_t.
>
> > Source/WebKit2/UIProcess/WebContext.cpp:820
> > + statistics->set("JavaScriptProtectedObjectTypeCounts", RefPtr<MutableDictionary>(createDictionaryFromHashMap(statisticsData.javaScriptProtectedObjectTypeCounts)).get());
>
> The RefPtr car should no be necessary.
Removed RefPtr wrap.
Thanks for reviewing!
Created attachment 105860[details]
Gather JavaScript, FastMalloc, icon, font, and glyph page statistics in WebProcess::getWebCoreStatistics()
This time with Windows build fix!
Comment on attachment 105986[details]
Gather memory cache statistics in WebProcess::getWebCoreStatistics()
View in context: https://bugs.webkit.org/attachment.cgi?id=105986&action=review> Source/WebKit2/WebProcess/WebProcess.cpp:852
> +#if ENABLE(XSLT)
> + counts.set(xslString, memoryCacheStatistics.xslStyleSheets.count);
> +#else
> + counts.set(xslString, 0);
> +#endif
It would cut down on ifdefs if the structure always had this field but all the numbers were zeroes.
Attachment 105986[details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1
Source/WebKit2/ChangeLog:3: ChangeLog entry has no bug number [changelog/bugnumber] [5]
Total errors found: 1 in 5 files
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #13)
> (From update of attachment 105986[details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=105986&action=review
>
> > Source/WebKit2/WebProcess/WebProcess.cpp:852
> > +#if ENABLE(XSLT)
> > + counts.set(xslString, memoryCacheStatistics.xslStyleSheets.count);
> > +#else
> > + counts.set(xslString, 0);
> > +#endif
>
> It would cut down on ifdefs if the structure always had this field but all the numbers were zeroes.
Very true! I'll look into doing that in a followup patch.
Attachment 106033[details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebKit2/ChangeLog:3: ChangeLog entry has no bug number [changelog/bugnumber] [5]
Total errors found: 1 in 2 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 106033[details]
Cut down ifdefs in WebKit::getWebCoreMemoryCacheStatistics()
View in context: https://bugs.webkit.org/attachment.cgi?id=106033&action=review>> Source/WebCore/loader/cache/MemoryCache.h:102
>> TypeStatistic xslStyleSheets;
>
> Is there any code anywhere that needs to be changed to initialize this to zero?
TypeStatistic has a default constructor that initializes everything to zero.
2011-08-29 20:57 PDT, Ada Chan
2011-08-29 22:45 PDT, Ada Chan
2011-08-30 12:56 PDT, Ada Chan
2011-08-31 12:26 PDT, Ada Chan
2011-08-31 16:53 PDT, Ada Chan
2011-09-01 10:20 PDT, Ada Chan
2011-09-01 14:42 PDT, Ada Chan