[Webkit-unassigned] [Bug 91214] [EFL] Add WebMemorySampler feature.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=91214





--- Comment #37 from JungJik Lee <jungjik.lee at samsung.com>  2012-08-27 22:13:37 PST ---
(From update of attachment 160672)
View in context: https://bugs.webkit.org/attachment.cgi?id=160672&action=review

>> Source/WebKit2/PlatformEfl.cmake:24
>> +    Shared/WebMemorySampler.h
> 
> CMake doesn't include .h file.

done, thanks your comment.

>> Source/WebKit2/PlatformEfl.cmake:189
>> +    -DENABLE_MEMORY_SAMPLER=1
> 
> This is not needed if you want to add it in OptionsTizen.cmake.

done. thanks.

>> Source/WebKit2/Shared/efl/WebMemorySamplerEfl.cpp:56
>> +const int maxBuffer = 128;
> 
> Do you want it to be exported? if not should be static or inside a nameless namespace.

done.

>> Source/WebKit2/Shared/efl/WebMemorySamplerEfl.cpp:59
>> +inline String getToken(FILE* file)
> 
> ditto. Even though 'inline' may have similar qualities as 'static' this behavior is compiler(and even compiler setting)-specific.

done.

>>> Source/WebKit2/Shared/efl/WebMemorySamplerEfl.cpp:71
>>> +inline void appendMemoryStats(WebMemoryStatistics& webKitMemoryStats, String key, size_t value)
>> 
>> const String& key
> 
> ditto.

done.

>> Source/WebKit2/Shared/efl/WebMemorySamplerEfl.cpp:77
>> +void sampleSystemMemoryInfo(WebMemoryStatistics& statics)
> 
> ditto.

thanks your comments.

>> Source/WebKit2/Shared/efl/WebMemorySamplerEfl.cpp:106
>> +    if (fMemoryStatus) {
> 
> empty line is preffered.

done.

>> Source/WebKit2/Shared/efl/WebMemorySamplerEfl.cpp:118
>> +    return applicationStats;
> 
> need empty line.

done.

>> Source/WebKit2/Shared/efl/WebMemorySamplerEfl.cpp:134
>> +    return String();
> 
> need empty line.

done.

>> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:748
>> +#endif
> 
> Is this related to context? I think that it should be in ewk_context.

it's better to stay in view. because memory sampler starts when view is created. I think it's nothing related with context. though it's using it.

>> Source/cmake/OptionsEfl.cmake:81
>> +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_MEMORY_SAMPLER ON)
> 
> I am not sure whether we can add this.
> But I think that we are using GLIB_SUPPORT for similar reason, 
> So it can be reasonable although this option is only WebKit2/MAC and WebKit2/Efl now.
> 
> Anyway, to add this, you should also put this in WebKitFeatures.cmake and cmakeconfig.cmake.

following your comment, I added defines to cmakeconfig.cmake and WebKitFeatures.cmake.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.