[Webkit-unassigned] [Bug 159346] [GLib] Use a GSource instead of a thread to poll memory pressure eventFD in linux implementation

[bugzilla-daemon at webkit.org]

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

--- Comment #7 from Antonio Gomes <tonikitoo at webkit.org> ---
Comment on attachment 283522
  --> https://bugs.webkit.org/attachment.cgi?id=283522
Updated patch

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

> Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp:108
> +    EventFDSource* eventFDSource = reinterpret_cast<EventFDSource*>(m_source.get());

that reinterpret_cast was a bit tricky to understand, but Zan cleared it up (line 101 allocates m_source with "sizeof(EventFDSource)".

> Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp:145
> +        LOG(MemoryPressure, "Invalidate eventfd.");

s/Invalidate/Invalid/g

> Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp:230
> +    m_eventFDPoller = std::make_unique<EventFDPoller>(m_eventFD.value(), [this] {

So my understand is that m_eventFDPoller is an alternative memory pressure source that works independent from the existing 'cgroup' solution, right?

However, it gets initialized after the cgroup initialization stuff (lines 209 and 215). So, if cgroup fails to initialize - which happens in most cases - then ::install bails out earlier (lines 218 and 225) and line 230 is not executed.

Should line 230 be our first option, and cgroup the second?

> Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp:234
> +        // FIXME: Current memcg does not provide any way for users to know how serious the memory pressure is.
> +        // So we assume all notifications from memcg are critical for now. If memcg had better inferfaces
> +        // to get a detailed memory pressure level in the future, we should update here accordingly.
> +        bool critical = true;

I believe a similar comment could be in MemoryPressureHandler::EventFDPoller::readAndNotify(), before we call m_nofityHandler(); Maybe a reduced version of it?

-- 
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/20160713/ca82ca3d/attachment.html>