[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

--- Comment #40 from Antonio Gomes <tonikitoo at webkit.org> ---
Comment on attachment 283624
  --> https://bugs.webkit.org/attachment.cgi?id=283624
Try to fix builds

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

> Source/WebKit2/UIProcess/linux/MemoryPressureMonitor.cpp:29
> +#if OS(LINUX)

is OS(LINUX) needed given that we are within Source/WebKit2/UIProcess/*linux*/...?

> Source/WebKit2/UIProcess/linux/MemoryPressureMonitor.cpp:128
> +static size_t calculateMemoryAvailable(size_t memoryFree, size_t activeFile, size_t inactiveFile, size_t slabReclaimable)
> +{
> +    if (memoryFree == notSet || activeFile == notSet || inactiveFile == notSet || slabReclaimable == notSet)
> +        return notSet;
> +
> +    size_t lowWatermark = lowWatermarkPages();
> +    if (lowWatermark == notSet)
> +        return notSet;
> +
> +    lowWatermark *= systemPageSize() / KB;
> +
> +    // Estimate the amount of memory available for userspace allocations, without causing swapping.
> +    // Free memory cannot be taken below the low watermark, before the system starts swapping.
> +    lowWatermark *= systemPageSize() / KB;
> +    size_t memoryAvailable = memoryFree - lowWatermark;
> +
> +    // Not all the page cache can be freed, otherwise the system will start swapping. Assume at least
> +    // half of the page cache, or the low watermark worth of cache, needs to stay.
> +    size_t pageCache = activeFile + inactiveFile;
> +    pageCache -= std::min(pageCache / 2, lowWatermark);
> +    memoryAvailable += pageCache;
> +
> +    // Part of the reclaimable slab consists of items that are in use, and cannot be freed.
> +    // Cap this estimate at the low watermark.
> +    memoryAvailable += slabReclaimable - std::min(slabReclaimable / 2, lowWatermark);
> +    return memoryAvailable;

Getting late to the party here, but is it really worth to to add support for kernels older than 2014/January? this adds some complexity to the patch that I would rather avoid if possible.

> Source/WebKit2/UIProcess/linux/MemoryPressureMonitor.cpp:186
> +    if (memoryAvailable == notSet) {

I believe most recent kernels have "MemAvailable" in /proc/meminfo.

So cant we shortcut it here by only reading "MemFree", "Active(file)", "Inactive(file)" and "SReclaimable" only if mem == notSet?

So move part of the switch above (lines 150 - 176) to with the "if" line 186.

Also, we could name it explicitly like "readProcMemInfoEntriesForAncientKernels" or so.

> Source/WebKit2/UIProcess/linux/MemoryPressureMonitor.cpp:227
> +bool MemoryPressureMonitor::isEnabled()

const?

-- 
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/20160718/e97ad3e9/attachment.html>