[Webkit-unassigned] [Bug 155255] MemoryPressureHandler doesn't work if cgroups aren't present in Linux

[bugzilla-daemon at webkit.org]

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

Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #273468|review?                     |review-
              Flags|                            |

--- Comment #4 from Martin Robinson <mrobinson at webkit.org> ---
Comment on attachment 273468
  --> https://bugs.webkit.org/attachment.cgi?id=273468
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=273468&action=review

Looks good, but I think it needs a little bit of cleanup. Thanks!

> Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp:54
> +static const unsigned s_pollTimeSec = 1;

We try to avoid abbreviated variable names in WebKit. How about s_pollingIntervalInSeconds?

> Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp:56
> +static const size_t s_memCriticalLimit = 100 * KB * KB; // 100 MB
> +static const size_t s_memNonCriticalLimit = 300 * KB * KB; // 300 MB

memory isn't abbreviated below, so it probably shouldn't be abbreviated here.

> Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp:118
> +        size_t memFree = static_cast<size_t>(-1); // bytes

I think this comment could be expanded. What does "bytes" signify here. Comments in WebKit should be complete sentences with periods. If you are trying to say that this value is stored in bytes, a better method would simply be to call this variable "memoryFreeInBytes."

> Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp:135
> +            bool critical = memFree < s_memCriticalLimit;

Maybe better to call this underMemoryPressure or something similar.

> Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp:210
> +    } while (false);

Probably better to use a helper function here rather than do { } while.

> Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp:225
> +        do {
> +            m_threadID = createThread(pollMemoryPressure, this, "WebCore: MemoryPressureHandler");
> +            if (!m_threadID) {
> +                logErrorAndCloseFDs("Failed to create a thread for MemoryPressureHandler");
> +                break;
> +            }
> +
> +            LOG(MemoryPressure, "Vmstat memory pressure handler installed.");
> +            vmstatPressureHandlerOk = true;
> +        } while (false);

Probably better to use a helper function here rather than do { } while.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160311/d3a22b92/attachment-0001.html>