Attachment 346848 Details for Bug 188444 – patch

[patch] patch

c-backup.diff (text/plain), 10.24 KB, created by Saam Barati on 2018-08-09 11:14:50 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2018-08-09 11:14:50 PDT

Size: 10.24 KB

patch

obsolete

>Index: Source/WTF/ChangeLog
>===================================================================
>--- Source/WTF/ChangeLog	(revision 234728)
>+++ Source/WTF/ChangeLog	(working copy)
>@@ -1,3 +1,37 @@
>+2018-08-09  Saam Barati  <sbarati@apple.com>
>+
>+        memoryFootprint should return size_t not optional<size_t>
>+        https://bugs.webkit.org/show_bug.cgi?id=188444
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We're now going to return zero instead of returning nullopt on failure.
>+        There was a lot of code dancing around memoryFootprint failing for no
>+        good reason.
>+        
>+        Users of this API were previously doing this on failure:
>+        - Treating it as zero (this was the most common user).
>+        - Crashing.
>+        - Bailing out early and not changing our memory pressure state. This change
>+        has the effect that instead of not changing our memory pressure state on
>+        failure, we will go back to thinking we're not under memory pressure. Since
>+        we relied on this API not failing to do anything useful (like kill the process
>+        or release memory), this won't change our behavior here in a meaningful way.
>+
>+        * wtf/MemoryFootprint.h:
>+        * wtf/MemoryPressureHandler.cpp:
>+        (WTF::MemoryPressureHandler::currentMemoryUsagePolicy):
>+        (WTF::MemoryPressureHandler::shrinkOrDie):
>+        (WTF::MemoryPressureHandler::measurementTimerFired):
>+        * wtf/cocoa/MemoryFootprintCocoa.cpp:
>+        (WTF::memoryFootprint):
>+        * wtf/linux/MemoryFootprintLinux.cpp:
>+        (WTF::memoryFootprint):
>+        * wtf/linux/MemoryPressureHandlerLinux.cpp:
>+        (WTF::MemoryPressureHandler::ReliefLogger::platformMemoryUsage):
>+        * wtf/win/MemoryFootprintWin.cpp:
>+        (WTF::memoryFootprint):
>+
> 2018-08-05  Darin Adler  <darin@apple.com>
> 
>         [Cocoa] More tweaks and refactoring to prepare for ARC
>Index: Source/WTF/wtf/MemoryFootprint.h
>===================================================================
>--- Source/WTF/wtf/MemoryFootprint.h	(revision 234727)
>+++ Source/WTF/wtf/MemoryFootprint.h	(working copy)
>@@ -29,7 +29,7 @@
> 
> namespace WTF {
> 
>-WTF_EXPORT_PRIVATE std::optional<size_t> memoryFootprint();
>+WTF_EXPORT_PRIVATE size_t memoryFootprint();
> 
> }
> 
>Index: Source/WTF/wtf/MemoryPressureHandler.cpp
>===================================================================
>--- Source/WTF/wtf/MemoryPressureHandler.cpp	(revision 234727)
>+++ Source/WTF/wtf/MemoryPressureHandler.cpp	(working copy)
>@@ -146,7 +146,7 @@ static MemoryUsagePolicy policyForFootpr
> 
> MemoryUsagePolicy MemoryPressureHandler::currentMemoryUsagePolicy()
> {
>-    return policyForFootprint(memoryFootprint().value_or(0));
>+    return policyForFootprint(memoryFootprint());
> }
> 
> void MemoryPressureHandler::shrinkOrDie()
>@@ -154,17 +154,16 @@ void MemoryPressureHandler::shrinkOrDie(
>     RELEASE_LOG(MemoryPressure, "Process is above the memory kill threshold. Trying to shrink down.");
>     releaseMemory(Critical::Yes, Synchronous::Yes);
> 
>-    auto footprint = memoryFootprint();
>-    RELEASE_ASSERT(footprint);
>-    RELEASE_LOG(MemoryPressure, "New memory footprint: %zu MB", footprint.value() / MB);
>+    size_t footprint = memoryFootprint();
>+    RELEASE_LOG(MemoryPressure, "New memory footprint: %zu MB", footprint / MB);
> 
>-    if (footprint.value() < thresholdForMemoryKill()) {
>+    if (footprint < thresholdForMemoryKill()) {
>         RELEASE_LOG(MemoryPressure, "Shrank below memory kill threshold. Process gets to live.");
>-        setMemoryUsagePolicyBasedOnFootprint(footprint.value());
>+        setMemoryUsagePolicyBasedOnFootprint(footprint);
>         return;
>     }
> 
>-    WTFLogAlways("Unable to shrink memory footprint of process (%zu MB) below the kill thresold (%zu MB). Killed\n", footprint.value() / MB, thresholdForMemoryKill() / MB);
>+    WTFLogAlways("Unable to shrink memory footprint of process (%zu MB) below the kill thresold (%zu MB). Killed\n", footprint / MB, thresholdForMemoryKill() / MB);
>     RELEASE_ASSERT(m_memoryKillCallback);
>     m_memoryKillCallback();
> }
>@@ -182,17 +181,14 @@ void MemoryPressureHandler::setMemoryUsa
> 
> void MemoryPressureHandler::measurementTimerFired()
> {
>-    auto footprint = memoryFootprint();
>-    if (!footprint)
>-        return;
>-
>-    RELEASE_LOG(MemoryPressure, "Current memory footprint: %zu MB", footprint.value() / MB);
>-    if (footprint.value() >= thresholdForMemoryKill()) {
>+    size_t footprint = memoryFootprint();
>+    RELEASE_LOG(MemoryPressure, "Current memory footprint: %zu MB", footprint / MB);
>+    if (footprint >= thresholdForMemoryKill()) {
>         shrinkOrDie();
>         return;
>     }
> 
>-    setMemoryUsagePolicyBasedOnFootprint(footprint.value());
>+    setMemoryUsagePolicyBasedOnFootprint(footprint);
> 
>     switch (m_memoryUsagePolicy) {
>     case MemoryUsagePolicy::Unrestricted:
>@@ -205,7 +201,7 @@ void MemoryPressureHandler::measurementT
>         break;
>     }
> 
>-    if (processState() == WebsamProcessState::Active && footprint.value() > thresholdForMemoryKillWithProcessState(WebsamProcessState::Inactive, m_pageCount))
>+    if (processState() == WebsamProcessState::Active && footprint > thresholdForMemoryKillWithProcessState(WebsamProcessState::Inactive, m_pageCount))
>         doesExceedInactiveLimitWhileActive();
>     else
>         doesNotExceedInactiveLimitWhileActive();
>Index: Source/WTF/wtf/cocoa/MemoryFootprintCocoa.cpp
>===================================================================
>--- Source/WTF/wtf/cocoa/MemoryFootprintCocoa.cpp	(revision 234727)
>+++ Source/WTF/wtf/cocoa/MemoryFootprintCocoa.cpp	(working copy)
>@@ -31,13 +31,13 @@
> 
> namespace WTF {
> 
>-std::optional<size_t> memoryFootprint()
>+size_t memoryFootprint()
> {
>     task_vm_info_data_t vmInfo;
>     mach_msg_type_number_t count = TASK_VM_INFO_COUNT;
>     kern_return_t result = task_info(mach_task_self(), TASK_VM_INFO, (task_info_t) &vmInfo, &count);
>     if (result != KERN_SUCCESS)
>-        return std::nullopt;
>+        return 0;
>     return static_cast<size_t>(vmInfo.phys_footprint);
> }
> 
>Index: Source/WTF/wtf/linux/MemoryFootprintLinux.cpp
>===================================================================
>--- Source/WTF/wtf/linux/MemoryFootprintLinux.cpp	(revision 234727)
>+++ Source/WTF/wtf/linux/MemoryFootprintLinux.cpp	(working copy)
>@@ -47,12 +47,12 @@ static void forEachLine(FILE* file, Func
> }
> #endif
> 
>-std::optional<size_t> memoryFootprint()
>+size_t memoryFootprint()
> {
> #if OS(LINUX)
>     FILE* file = fopen("/proc/self/smaps", "r");
>     if (!file)
>-        return std::nullopt;
>+        return 0;
> 
>     unsigned long totalPrivateDirtyInKB = 0;
>     bool isAnonymous = false;
>@@ -87,7 +87,7 @@ std::optional<size_t> memoryFootprint()
>     fclose(file);
>     return totalPrivateDirtyInKB * KB;
> #endif
>-    return std::nullopt;
>+    return 0;
> }
> 
> }
>Index: Source/WTF/wtf/linux/MemoryPressureHandlerLinux.cpp
>===================================================================
>--- Source/WTF/wtf/linux/MemoryPressureHandlerLinux.cpp	(revision 234727)
>+++ Source/WTF/wtf/linux/MemoryPressureHandlerLinux.cpp	(working copy)
>@@ -135,11 +135,7 @@ void MemoryPressureHandler::platformRele
> 
> std::optional<MemoryPressureHandler::ReliefLogger::MemoryUsage> MemoryPressureHandler::ReliefLogger::platformMemoryUsage()
> {
>-    size_t physical = 0;
>-    auto footprint = memoryFootprint();
>-    if (footprint)
>-        physical = footprint.value();
>-    return MemoryUsage {processMemoryUsage(), physical};
>+    return MemoryUsage {processMemoryUsage(), memoryFootprint()};
> }
> 
> 
>Index: Source/WTF/wtf/win/MemoryFootprintWin.cpp
>===================================================================
>--- Source/WTF/wtf/win/MemoryFootprintWin.cpp	(revision 234727)
>+++ Source/WTF/wtf/win/MemoryFootprintWin.cpp	(working copy)
>@@ -35,7 +35,7 @@
> 
> namespace WTF {
> 
>-std::optional<size_t> memoryFootprint()
>+size_t memoryFootprint()
> {
>     // We would like to calculate size of private working set.
>     // https://msdn.microsoft.com/en-us/library/windows/desktop/ms684891(v=vs.85).aspx
>@@ -46,7 +46,7 @@ std::optional<size_t> memoryFootprint()
>     // > memory demand increases.
>     Win32Handle process(OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, GetCurrentProcessId()));
>     if (!process.isValid())
>-        return std::nullopt;
>+        return 0;
> 
>     auto countSizeOfPrivateWorkingSet = [] (const PSAPI_WORKING_SET_INFORMATION& workingSets) {
>         constexpr const size_t pageSize = 4 * KB;
>@@ -81,7 +81,7 @@ std::optional<size_t> memoryFootprint()
>             return countSizeOfPrivateWorkingSet(*workingSets);
> 
>         if (GetLastError() != ERROR_BAD_LENGTH)
>-            return std::nullopt;
>+            return 0;
>         numberOfEntries = updateNumberOfEntries(workingSets->NumberOfEntries);
>     }
> }
>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 234727)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,13 @@
>+2018-08-09  Saam Barati  <sbarati@apple.com>
>+
>+        memoryFootprint should return size_t not optional<size_t>
>+        https://bugs.webkit.org/show_bug.cgi?id=188444
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * page/cocoa/ResourceUsageOverlayCocoa.mm:
>+        (WebCore::ResourceUsageOverlay::platformDraw):
>+
> 2018-08-09  Charlie Turner  <cturner@igalia.com>
> 
>         Fix copyright headers on new ISO parsing class
>Index: Source/WebCore/page/cocoa/ResourceUsageOverlayCocoa.mm
>===================================================================
>--- Source/WebCore/page/cocoa/ResourceUsageOverlayCocoa.mm	(revision 234727)
>+++ Source/WebCore/page/cocoa/ResourceUsageOverlayCocoa.mm	(working copy)
>@@ -458,10 +458,7 @@ void ResourceUsageOverlay::platformDraw(
> 
>     static CGColorRef colorForLabels = createColor(0.9, 0.9, 0.9, 1);
>     showText(context, 10, 20, colorForLabels, String::format("        CPU: %g", data.cpu.last()));
>-    if (auto footprint = memoryFootprint())
>-        showText(context, 10, 30, colorForLabels, "  Footprint: " + formatByteNumber(*footprint));
>-    else
>-        showText(context, 10, 30, colorForLabels, "  Footprint: " + formatByteNumber(data.totalDirtySize.last()));
>+    showText(context, 10, 30, colorForLabels, "  Footprint: " + formatByteNumber(memoryFootprint()));
>     showText(context, 10, 40, colorForLabels, "   External: " + formatByteNumber(data.totalExternalSize.last()));
> 
>     float y = 55;

Actions: View | Formatted Diff | Diff

Attachments on bug 188444: 346848