Attachment 342489 Details for Bug 186467 – Patch

[patch] Patch

bug-186467-20180611170744.patch (text/plain), 6.75 KB, created by Keith Miller on 2018-06-11 17:07:44 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Keith Miller

Created: 2018-06-11 17:07:44 PDT

Size: 6.75 KB

patch

obsolete

>Subversion Revision: 232723
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index f1551768b6c61fc3cdca8224594df5210996b619..873cb2b8f99eac11996322b4208bc0a296325a0e 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2018-06-11  Keith Miller  <keith_miller@apple.com>
>+
>+        Loading cnn.com in MiniBrowser hits Structure::dump() under DFG::AdaptiveInferredPropertyValueWatchpoint::handleFire  which churns 65KB of memory
>+        https://bugs.webkit.org/show_bug.cgi?id=186467
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch adds a LazyFireDetail that wraps ScopedLambda so that
>+        we don't actually malloc any strings for firing unless those
>+        Strings are actually going to be printed.
>+
>+        * bytecode/Watchpoint.h:
>+        (JSC::LazyFireDetail::LazyFireDetail):
>+        * dfg/DFGAdaptiveInferredPropertyValueWatchpoint.cpp:
>+        (JSC::DFG::AdaptiveInferredPropertyValueWatchpoint::handleFire):
>+        * dfg/DFGAdaptiveStructureWatchpoint.cpp:
>+        (JSC::DFG::AdaptiveStructureWatchpoint::fireInternal):
>+        * runtime/ArrayPrototype.cpp:
>+        (JSC::ArrayPrototypeAdaptiveInferredPropertyWatchpoint::handleFire):
>+        * runtime/Options.h:
>+
> 2018-06-11  Michael Saboff  <msaboff@apple.com>
> 
>         JavaScriptCore: Disable 32-bit JIT on Windows
>diff --git a/Source/JavaScriptCore/bytecode/Watchpoint.h b/Source/JavaScriptCore/bytecode/Watchpoint.h
>index 4476e2a7b5135ea7dbcf2ec688c7397b72900e01..778fe16c3828ad224cbc1e3cdf0093d0a66f8788 100644
>--- a/Source/JavaScriptCore/bytecode/Watchpoint.h
>+++ b/Source/JavaScriptCore/bytecode/Watchpoint.h
>@@ -29,6 +29,7 @@
> #include <wtf/FastMalloc.h>
> #include <wtf/Noncopyable.h>
> #include <wtf/PrintStream.h>
>+#include <wtf/ScopedLambda.h>
> #include <wtf/SentinelLinkedList.h>
> #include <wtf/ThreadSafeRefCounted.h>
> 
>@@ -62,6 +63,28 @@ private:
>     const char* m_string;
> };
> 
>+template<typename... Types>
>+class LazyFireDetail : public FireDetail {
>+public:
>+    LazyFireDetail(const Types&... args)
>+    {
>+        m_lambda = scopedLambda<void(PrintStream&)>([&] (PrintStream& out) {
>+            out.print(args...);
>+        });
>+    }
>+
>+    void dump(PrintStream& out) const override { m_lambda(out); }
>+
>+private:
>+    ScopedLambda<void(PrintStream&)> m_lambda;
>+};
>+
>+template<typename... Types>
>+LazyFireDetail<Types...> createLazyFireDetail(const Types&... types)
>+{
>+    return LazyFireDetail<Types...>(types...);
>+}
>+
> class WatchpointSet;
> 
> class Watchpoint : public BasicRawSentinelNode<Watchpoint> {
>diff --git a/Source/JavaScriptCore/dfg/DFGAdaptiveInferredPropertyValueWatchpoint.cpp b/Source/JavaScriptCore/dfg/DFGAdaptiveInferredPropertyValueWatchpoint.cpp
>index 207b2fcecaf4412facbf1da2b22b41812974d355..f7b91c8d758429b1319169e925a4add3cd2b9dd4 100644
>--- a/Source/JavaScriptCore/dfg/DFGAdaptiveInferredPropertyValueWatchpoint.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGAdaptiveInferredPropertyValueWatchpoint.cpp
>@@ -46,12 +46,9 @@ void AdaptiveInferredPropertyValueWatchpoint::handleFire(const FireDetail& detai
>         dataLog("Firing watchpoint ", RawPointer(this), " (", key(), ") on ", *m_codeBlock, "\n");
> 
> 
>-    StringPrintStream out;
>-    out.print("Adaptation of ", key(), " failed: ", detail);
>+    auto lazyDetail = createLazyFireDetail("Adaptation of ", key(), " failed: ", detail);
> 
>-    StringFireDetail stringDetail(out.toCString().data());
>-
>-    m_codeBlock->jettison(Profiler::JettisonDueToUnprofiledWatchpoint, CountReoptimization, &stringDetail);
>+    m_codeBlock->jettison(Profiler::JettisonDueToUnprofiledWatchpoint, CountReoptimization, &lazyDetail);
> }
> 
> } } // namespace JSC::DFG
>diff --git a/Source/JavaScriptCore/dfg/DFGAdaptiveStructureWatchpoint.cpp b/Source/JavaScriptCore/dfg/DFGAdaptiveStructureWatchpoint.cpp
>index 0854c56b0764365f60c022bb0eb21784caa3f37b..599818af10b29f720d8febb83c5400bed6172b32 100644
>--- a/Source/JavaScriptCore/dfg/DFGAdaptiveStructureWatchpoint.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGAdaptiveStructureWatchpoint.cpp
>@@ -57,18 +57,15 @@ void AdaptiveStructureWatchpoint::fireInternal(const FireDetail& detail)
>         return;
>     }
>     
>-    if (DFG::shouldDumpDisassembly()) {
>+    if (Options::verboseWatchpointFire()) {
>         dataLog(
>             "Firing watchpoint ", RawPointer(this), " (", m_key, ") on ", *m_codeBlock, "\n");
>     }
>-    
>-    StringPrintStream out;
>-    out.print("Adaptation of ", m_key, " failed: ", detail);
>-    
>-    StringFireDetail stringDetail(out.toCString().data());
>-    
>+
>+    auto lazyDetail = createLazyFireDetail("Adaptation of ", m_key, " failed: ", detail);
>+
>     m_codeBlock->jettison(
>-        Profiler::JettisonDueToUnprofiledWatchpoint, CountReoptimization, &stringDetail);
>+        Profiler::JettisonDueToUnprofiledWatchpoint, CountReoptimization, &lazyDetail);
> }
> 
> } } // namespace JSC::DFG
>diff --git a/Source/JavaScriptCore/runtime/ArrayPrototype.cpp b/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
>index d85f7106fce0bbdc98e7f2f2fce902ce4bb897c8..241f240e01e414475ba1723f6ad4ad8cb34fba3b 100644
>--- a/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
>+++ b/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
>@@ -1514,16 +1514,13 @@ ArrayPrototypeAdaptiveInferredPropertyWatchpoint::ArrayPrototypeAdaptiveInferred
> 
> void ArrayPrototypeAdaptiveInferredPropertyWatchpoint::handleFire(const FireDetail& detail)
> {
>-    StringPrintStream out;
>-    out.print("ArrayPrototype adaption of ", key(), " failed: ", detail);
>-
>-    StringFireDetail stringDetail(out.toCString().data());
>+    auto lazyDetail = createLazyFireDetail("ArrayPrototype adaption of ", key(), " failed: ", detail);
> 
>     if (ArrayPrototypeInternal::verbose)
>-        WTF::dataLog(stringDetail, "\n");
>+        WTF::dataLog(lazyDetail, "\n");
> 
>     JSGlobalObject* globalObject = m_arrayPrototype->globalObject();
>-    globalObject->arraySpeciesWatchpoint().fireAll(globalObject->vm(), stringDetail);
>+    globalObject->arraySpeciesWatchpoint().fireAll(globalObject->vm(), lazyDetail);
> }
> 
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/runtime/Options.h b/Source/JavaScriptCore/runtime/Options.h
>index 7e8c954f0abe533a3c7b25f2a890eaa404b25e01..3ba69513294e655cbd308cb4a26cc4587a11ba05 100644
>--- a/Source/JavaScriptCore/runtime/Options.h
>+++ b/Source/JavaScriptCore/runtime/Options.h
>@@ -219,6 +219,7 @@ constexpr bool enableWebAssemblyStreamingApi = false;
>     v(bool, verboseDFGFailure, false, Normal, nullptr) \
>     v(bool, verboseFTLToJSThunk, false, Normal, nullptr) \
>     v(bool, verboseFTLFailure, false, Normal, nullptr) \
>+    v(bool, verboseWatchpointFire, false, Normal, "dump information about when watchpoints are fired") \
>     v(bool, alwaysComputeHash, false, Normal, nullptr) \
>     v(bool, testTheFTL, false, Normal, nullptr) \
>     v(bool, verboseSanitizeStack, false, Normal, nullptr) \

Flags:

simon.fraser: review+
commit-queue: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 186467: 342489 | 342497