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

[bugzilla-daemon at webkit.org]

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


Gyuyoung Kim <gyuyoung.kim at samsung.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #158812|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #14 from Gyuyoung Kim <gyuyoung.kim at samsung.com>  2012-08-16 08:52:29 PST ---
(From update of attachment 158812)
View in context: https://bugs.webkit.org/attachment.cgi?id=158812&action=review

First of all, I'm not sure if we can support memory usage functionality as APIs. Can't we support this functionality when we set environment variable ? In addition, I think this patch needs some loves. So. I set r-. In addition,

> Source/JavaScriptCore/PlatformEfl.cmake:20
> +IF (ENABLE_MEMORY_SAMPLER)

ENABLE_XXX macros were removed to avoid duplicated #ifdef guard. However, though MemoryStatics.cpp file is not guarded by #if ENABLE(XXX), I would like to recommend to add the MemoryStatistics.cpp to existing JavaScriptCore_SOURCES.

> Source/WTF/wtf/Platform.h:1152
> +#if PLATFORM(EFL)

We don't need to add this macro to here.

> Source/WebKit2/PlatformEfl.cmake:26
> +    Shared/efl/WebMemorySamplerEfl.cpp

Please move this line to below block.

> Source/WebKit2/Shared/efl/WebMemorySamplerEfl.cpp:24
> + *

Remove unneeded line.

> Source/WebKit2/Shared/efl/WebMemorySamplerEfl.cpp:55
> +} ApplicationMemoryStats;

Duplicated struct name declaration ?

> Source/WebKit2/Shared/efl/WebMemorySamplerEfl.cpp:63
> +        return String();

Please add a new line after return.

> Source/WebKit2/Shared/efl/WebMemorySamplerEfl.cpp:66
> +    fscanf(file, "%s", buffer);

Please add a new line.

> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:238
> +    EINA_SAFETY_ON_NULL_RETURN(ewkContext);

EFL port has add an empty line after EINA_SAFETY_ON_NULL_RETURN macro.

> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:245
> +    EINA_SAFETY_ON_NULL_RETURN(ewkContext);

ditto.

> Source/WebKit2/UIProcess/API/efl/ewk_context.h:139
> +* Start memory sampler.

Add a space before *.

> Source/WebKit2/UIProcess/API/efl/ewk_context.h:147
> +EAPI void ewk_context_memory_sampler_start(Ewk_Context* context, double timer_interval);

Nit : Wrong '*' operator place.

> Source/WebKit2/UIProcess/API/efl/ewk_context.h:150
> +* Stop memory sampler.

ditto.

> Source/WebKit2/UIProcess/API/efl/ewk_context.h:154
> +EAPI void ewk_context_memory_sampler_stop(Ewk_Context* context);

Nit : Wrong '*' operator place.

> Source/cmake/OptionsEfl.cmake:55
> +SET(ENABLE_MEMORY_SAMPLER ON)

Please use WEBKIT_OPTION_DEFAULT_PORT_VALUE macro.

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