Attachment 370188 Details for Bug 197998 – Patch for landing

[patch] Patch for landing

bug-197998-20190518083222.patch (text/plain), 7.38 KB, created by Tadeu Zagallo on 2019-05-17 23:32:24 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2019-05-17 23:32:24 PDT

Size: 7.38 KB

patch

obsolete

>Subversion Revision: 245317
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 312863185dc8be5ea8d89570fd5919333e6c814d..5dc92b535062ff5d016dba664ec70184c06f7994 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-05-17  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        Add extra information to dumpJITMemory
>+        https://bugs.webkit.org/show_bug.cgi?id=197998
>+
>+        Reviewed by Saam Barati.
>+
>+        Add ktrace events around the memory dump and mach_absolute_time to link the
>+        events with the entries in the dump. Additionally, add a background queue
>+        to flush on a configurable interval, since the atexit callback does not work
>+        in every situation.
>+
>+        * jit/ExecutableAllocator.cpp:
>+        (JSC::dumpJITMemory):
>+        * runtime/Options.h:
>+
> 2019-05-14  Keith Miller  <keith_miller@apple.com>
> 
>         Fix issue with byteOffset on ARM64E
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index 6bc60aa7e4bf74aa275461c4ac185caa4d982601..de3bea7a61a1d800684f61a4d61523d607455cf5 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-05-17  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        Add extra information to dumpJITMemory
>+        https://bugs.webkit.org/show_bug.cgi?id=197998
>+
>+        Reviewed by Saam Barati.
>+
>+        Add a new trace point code for JSC::dumpJITMemory
>+
>+        * wtf/SystemTracing.h:
>+
> 2019-05-14  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r245281.
>diff --git a/Source/JavaScriptCore/jit/ExecutableAllocator.cpp b/Source/JavaScriptCore/jit/ExecutableAllocator.cpp
>index 7230cb7eef8c9f53cbb1a91248cbcc8ce06706eb..4be4b268e612e82498f492f893a6162a3d4eadfd 100644
>--- a/Source/JavaScriptCore/jit/ExecutableAllocator.cpp
>+++ b/Source/JavaScriptCore/jit/ExecutableAllocator.cpp
>@@ -33,8 +33,11 @@
> #include "JSCInlines.h"
> #include <wtf/MetaAllocator.h>
> #include <wtf/PageReservation.h>
>+#include <wtf/SystemTracing.h>
>+#include <wtf/WorkQueue.h>
> 
> #if OS(DARWIN)
>+#include <mach/mach_time.h>
> #include <sys/mman.h>
> #endif
> 
>@@ -559,39 +562,61 @@ void dumpJITMemory(const void* dst, const void* src, size_t size)
>     static uint8_t* buffer;
>     static constexpr size_t bufferSize = fixedExecutableMemoryPoolSize;
>     static size_t offset = 0;
>-    static auto flush = [] {
>+    static Lock dumpJITMemoryLock;
>+    static bool needsToFlush = false;
>+    static auto flush = [](const AbstractLocker&) {
>         if (fd == -1) {
>             fd = open(Options::dumpJITMemoryPath(), O_CREAT | O_TRUNC | O_APPEND | O_WRONLY | O_EXLOCK | O_NONBLOCK, 0666);
>             RELEASE_ASSERT(fd != -1);
>         }
>         write(fd, buffer, offset);
>         offset = 0;
>+        needsToFlush = false;
>     };
> 
>-    static Lock dumpJITMemoryLock;
>     static std::once_flag once;
>+    static LazyNeverDestroyed<Ref<WorkQueue>> flushQueue;
>     std::call_once(once, [] {
>         buffer = bitwise_cast<uint8_t*>(malloc(bufferSize));
>+        flushQueue.construct(WorkQueue::create("jsc.dumpJITMemory.queue", WorkQueue::Type::Serial, WorkQueue::QOS::Background));
>         std::atexit([] {
>             LockHolder locker(dumpJITMemoryLock);
>-            flush();
>+            flush(locker);
>             close(fd);
>+            fd = -1;
>         });
>     });
> 
>-    static auto write = [](const void* src, size_t size) {
>+    static auto enqueueFlush = [](const AbstractLocker&) {
>+        if (needsToFlush)
>+            return;
>+
>+        needsToFlush = true;
>+        flushQueue.get()->dispatchAfter(Seconds(Options::dumpJITMemoryFlushInterval()), [] {
>+            LockHolder locker(dumpJITMemoryLock);
>+            if (!needsToFlush)
>+                return;
>+            flush(locker);
>+        });
>+    };
>+
>+    static auto write = [](const AbstractLocker& locker, const void* src, size_t size) {
>         if (UNLIKELY(offset + size > bufferSize))
>-            flush();
>+            flush(locker);
>         memcpy(buffer + offset, src, size);
>         offset += size;
>+        enqueueFlush(locker);
>     };
> 
>     LockHolder locker(dumpJITMemoryLock);
>+    uint64_t time = mach_absolute_time();
>     uint64_t dst64 = bitwise_cast<uintptr_t>(dst);
>-    write(&dst64, sizeof(dst64));
>     uint64_t size64 = size;
>-    write(&size64, sizeof(size64));
>-    write(src, size);
>+    TraceScope(DumpJITMemoryStart, DumpJITMemoryStop, time, dst64, size64);
>+    write(locker, &time, sizeof(time));
>+    write(locker, &dst64, sizeof(dst64));
>+    write(locker, &size64, sizeof(size64));
>+    write(locker, src, size);
> #else
>     UNUSED_PARAM(dst);
>     UNUSED_PARAM(src);
>diff --git a/Source/JavaScriptCore/runtime/Options.h b/Source/JavaScriptCore/runtime/Options.h
>index 6bc150021a3a5003e8bb14a482ce1930562fc5d8..ddc8a347ed7ac715ec3cf2577952d2fb52641b4e 100644
>--- a/Source/JavaScriptCore/runtime/Options.h
>+++ b/Source/JavaScriptCore/runtime/Options.h
>@@ -518,6 +518,7 @@ constexpr bool enableWebAssemblyStreamingApi = false;
>     v(bool, validateAbstractInterpreterState, false, Restricted, nullptr) \
>     v(double, validateAbstractInterpreterStateProbability, 0.5, Normal, nullptr) \
>     v(optionString, dumpJITMemoryPath, nullptr, Restricted, nullptr) \
>+    v(double, dumpJITMemoryFlushInterval, 10, Restricted, "Maximum time in between flushes of the JIT memory dump in seconds.") \
> 
> 
> enum OptionEquivalence {
>diff --git a/Source/WTF/wtf/SystemTracing.h b/Source/WTF/wtf/SystemTracing.h
>index bcce0f9c77d157104d84f63e42cd50cbb2d7836c..75fdea986b8e92bd9821cab03d8f0fce56feb828 100644
>--- a/Source/WTF/wtf/SystemTracing.h
>+++ b/Source/WTF/wtf/SystemTracing.h
>@@ -47,6 +47,8 @@ enum TracePointCode {
>     WebAssemblyCompileEnd,
>     WebAssemblyExecuteStart,
>     WebAssemblyExecuteEnd,
>+    DumpJITMemoryStart,
>+    DumpJITMemoryStop,
> 
>     WebCoreRange = 5000,
>     MainResourceLoadDidStartProvisional,
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 40a60b502837efb3b0b6ea60ebf362d4c5cf61d9..241b11352145315bafc20c8c5e0ffdf1a306abbe 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-05-17  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        Add extra information to dumpJITMemory
>+        https://bugs.webkit.org/show_bug.cgi?id=197998
>+
>+        Reviewed by Saam Barati.
>+
>+        Add description for the new dumpJITMemory trace point code.
>+
>+        * Tracing/SystemTracePoints.plist:
>+
> 2019-05-14  Andy Estes  <aestes@apple.com>
> 
>         [Apple Pay] Payment APIs should be completely disabled in web views into which clients have injected user scripts
>diff --git a/Tools/Tracing/SystemTracePoints.plist b/Tools/Tracing/SystemTracePoints.plist
>index c140e91dd0f82d8be1e8e3e6af1da55fcd156304..ad281569215a0d8ad1892d223ae00e046788f5ea 100644
>--- a/Tools/Tracing/SystemTracePoints.plist
>+++ b/Tools/Tracing/SystemTracePoints.plist
>@@ -43,6 +43,18 @@
>                  <key>CodeEnd</key>
>                  <string>2506</string>
>              </dict>
>+             <dict>
>+                 <key>Name</key>
>+                 <string>Dump JIT Memory</string>
>+                 <key>Type</key>
>+                 <string>Interval</string>
>+                 <key>Component</key>
>+                 <string>47</string>
>+                 <key>CodeBegin</key>
>+                 <string>2507</string>
>+                 <key>CodeEnd</key>
>+                 <string>2508</string>
>+             </dict>
>              <dict>
>                  <key>Name</key>
>                  <string>Main Resource Load</string>

Actions: View | Formatted Diff | Diff

Attachments on bug 197998: 370142 | 370151 | 370188