Merge pull request #1554 from filipe-norte-red/wpe-2.46-gc-heap-snapshot-improvements

[wpe-2.46] Improve memory usage during GCHeap snapshot

Author: magomez

Source/JavaScriptCore/heap/HeapSnapshotBuilder.cpp

@@ -40,6 +40,81 @@
 
 namespace JSC {
 
+namespace {
+
+class FileOutputStringBuilder {
+    WTF_MAKE_NONCOPYABLE(FileOutputStringBuilder);
+
+    FileSystem::PlatformFileHandle m_fileHandle;
+    StringBuilder m_builder;
+    OverflowPolicy m_overflowPolicy;
+    bool m_overflowed { false };
+
+public:
+    FileOutputStringBuilder(FileSystem::PlatformFileHandle fileHandle, OverflowPolicy overflowPolicy)
+        : m_fileHandle(WTFMove(fileHandle))
+        , m_builder(overflowPolicy)
+        , m_overflowPolicy(overflowPolicy)
+    {
+    }
+
+    ~FileOutputStringBuilder()
+    {
+        flush(true);
+    }
+
+    template<typename... StringTypes> void append(StringTypes... fragment)
+    {
+        m_builder.append(fragment...);
+        flush();
+    }
+
+    void appendQuotedJSONString(const String& string)
+    {
+        m_builder.appendQuotedJSONString(string);
+        flush();
+    }
+
+    void flush(bool force = false)
+    {
+        constexpr size_t kBufferLengthLimit = 4 * WTF::KB;
+        if ((force && !m_builder.isEmpty()) || m_builder.length() >= kBufferLengthLimit)
+        {
+            CString utf8String = m_builder.toStringPreserveCapacity().utf8();
+            auto ret = FileSystem::writeToFile(m_fileHandle, utf8String.span());
+
+            if (ret < 0)
+            {
+                // A negative return value does not necessarily mean we ran out of disk space, but due the
+                // lack of a better indicator, we'll assume that is the case
+                m_overflowed = true;
+
+                if (m_overflowPolicy == OverflowPolicy::CrashOnOverflow)
+                {
+                    WTFLogAlways("Forcing crash (policy based) due lack of disk space in heap snapshot builder");
+                    CRASH();
+                }
+            }
+
+            m_builder.clear();
+            m_builder.reserveCapacity(kBufferLengthLimit);
+        }
+    }
+
+    bool hasOverflowed() const
+    {
+        return m_overflowed || m_builder.hasOverflowed();
+    }
+
+    void clear()
+    {
+        m_builder.clear();
+        FileSystem::truncateFile(m_fileHandle, 0);
+    }
+};
+
+} // namespace
+
 WTF_MAKE_TZONE_ALLOCATED_IMPL(HeapSnapshotBuilder);
 
 NodeIdentifier HeapSnapshotBuilder::nextAvailableObjectIdentifier = 1;
@@ -335,6 +410,20 @@ String HeapSnapshotBuilder::descriptionForCell(JSCell *cell) const
 }
 
 String HeapSnapshotBuilder::json(Function<bool (const HeapSnapshotNode&)> allowNodeCallback)
+{
+    StringBuilder json(m_overflowPolicy);
+    writeJson(WTFMove(allowNodeCallback), json);
+    return json.toString();
+}
+
+void HeapSnapshotBuilder::writeJsonToFile(FileSystem::PlatformFileHandle fileHandle)
+{
+    FileOutputStringBuilder json(fileHandle, m_overflowPolicy);
+    writeJson([] (const HeapSnapshotNode&) { return true; }, json);
+}
+
+template<typename FileOutputStringBuilder>
+void HeapSnapshotBuilder::writeJson(Function<bool (const HeapSnapshotNode&)>&& allowNodeCallback, FileOutputStringBuilder &json)
 {
     VM& vm = m_profiler.vm();
     DeferGCForAWhile deferGC(vm);
@@ -356,8 +445,6 @@ String HeapSnapshotBuilder::json(Function<bool (const HeapSnapshotNode&)> allowN
     UncheckedKeyHashMap<UniquedStringImpl*, unsigned> edgeNameIndexes;
     unsigned nextEdgeNameIndex = 0;
 
-    StringBuilder json(m_overflowPolicy);
-
     auto appendNodeJSON = [&] (const HeapSnapshotNode& node) {
         // Let the client decide if they want to allow or disallow certain nodes.
         if (!allowNodeCallback(node))
@@ -633,9 +720,9 @@ String HeapSnapshotBuilder::json(Function<bool (const HeapSnapshotNode&)> allowN
     json.append('}');
     if (json.hasOverflowed()) {
         m_hasOverflowed = true;
-        return { };
+
+        json.clear();
     }
-    return json.toString();
 }
 
 } // namespace JSC

Source/JavaScriptCore/heap/HeapSnapshotBuilder.h

@@ -33,6 +33,7 @@
 #include <wtf/OverflowPolicy.h>
 #include <wtf/TZoneMalloc.h>
 #include <wtf/Vector.h>
+#include <wtf/FileSystem.h>
 
 namespace JSC {
 
@@ -132,6 +133,7 @@ class JS_EXPORT_PRIVATE HeapSnapshotBuilder final : public HeapAnalyzer {
 
     String json();
     String json(Function<bool (const HeapSnapshotNode&)> allowNodeCallback);
+    void writeJsonToFile(FileSystem::PlatformFileHandle fileHandle);
 
     bool hasOverflowed() const { return m_hasOverflowed; }
 
@@ -145,6 +147,9 @@ class JS_EXPORT_PRIVATE HeapSnapshotBuilder final : public HeapAnalyzer {
 
     String descriptionForCell(JSCell*) const;
 
+    template<typename OutputStringBuilder>
+    void writeJson(Function<bool (const HeapSnapshotNode&)>&& allowNodeCallback, OutputStringBuilder &json);
+
     struct RootData {
         ASCIILiteral reachabilityFromOpaqueRootReasons;
         RootMarkReason markReason { RootMarkReason::None };

Source/WebCore/bindings/js/GCController.cpp

@@ -141,28 +141,43 @@ void GCController::deleteAllLinkedCode(DeleteAllCodeEffort effort)
 
 void GCController::dumpHeapForVM(VM& vm)
 {
-    auto [tempFilePath, fileHandle] = FileSystem::openTemporaryFile("GCHeap"_s);
+    static const char* sHeapDumpDirOverride = []() {
+        return getenv("WEBKIT_HEAP_SNAPSHOT_DIR");
+    }();
+
+    String tempFilePath;
+    FileSystem::PlatformFileHandle fileHandle;
+
+    if (sHeapDumpDirOverride) {
+        char buf[32];
+        time_t now = time(nullptr);
+        struct tm* tstruct = localtime(&now);
+        std::strftime(buf, sizeof(buf), "%Y%m%d_%H%M%S", tstruct);
+
+        tempFilePath = FileSystem::fileSystemRepresentation(
+            FileSystem::pathByAppendingComponent(WTF::String::fromUTF8(sHeapDumpDirOverride),
+                                                 makeString("GCHeap_"_s, WTF::String::fromUTF8(buf)))).span();
+        fileHandle = FileSystem::openFile(tempFilePath, FileSystem::FileOpenMode::ReadWrite);
+    } else {
+        std::tie(tempFilePath, fileHandle) = FileSystem::openTemporaryFile("GCHeap"_s);
+    }
+
     if (!FileSystem::isHandleValid(fileHandle)) {
-        WTFLogAlways("Dumping GC heap failed to open temporary file");
+        WTFLogAlways("Dumping GC heap failed to open temporary file: %s", tempFilePath.utf8().data());
         return;
     }
 
     JSLockHolder lock(vm);
     sanitizeStackForVM(vm);
 
-    String jsonData;
     {
         DeferGCForAWhile deferGC(vm); // Prevent concurrent GC from interfering with the full GC that the snapshot does.
 
         HeapSnapshotBuilder snapshotBuilder(vm.ensureHeapProfiler(), HeapSnapshotBuilder::SnapshotType::GCDebuggingSnapshot);
         snapshotBuilder.buildSnapshot();
-
-        jsonData = snapshotBuilder.json();
+        snapshotBuilder.writeJsonToFile(fileHandle);
     }
 
-    CString utf8String = jsonData.utf8();
-
-    FileSystem::writeToFile(fileHandle, utf8String.span());
     FileSystem::closeFile(fileHandle);
     WTFLogAlways("Dumped GC heap to %s%s", tempFilePath.utf8().data(), isMainThread() ? ""_s : " for Worker");
 }