Changeset 243702 in webkit

Timestamp:

Apr 1, 2019, 11:33:45 AM (7 years ago)

Author:

ysuzuki@apple.com

Message:

[JSC] JSRunLoopTimer::Manager should be small
​https://bugs.webkit.org/show_bug.cgi?id=196425

Reviewed by Darin Adler.

Using very large Key or Value in HashMap potentially bloats memory since HashMap pre-allocates large size of
memory ((sizeof(Key) + sizeof(Value)) * N) for its backing storage's array. Using std::unique_ptr<> for JSRunLoopTimer's
PerVMData to keep HashMap's backing store size small.

• runtime/JSRunLoopTimer.cpp:

(JSC::JSRunLoopTimer::Manager::timerDidFire):
(JSC::JSRunLoopTimer::Manager::registerVM):
(JSC::JSRunLoopTimer::Manager::scheduleTimer):
(JSC::JSRunLoopTimer::Manager::cancelTimer):
(JSC::JSRunLoopTimer::Manager::timeUntilFire):
(JSC::JSRunLoopTimer::Manager::didChangeRunLoop):

• runtime/JSRunLoopTimer.h:

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/JSRunLoopTimer.cpp (modified) (6 diffs)
• runtime/JSRunLoopTimer.h (modified) (3 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-04-01  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] JSRunLoopTimer::Manager should be small
+        https://bugs.webkit.org/show_bug.cgi?id=196425
+
+        Reviewed by Darin Adler.
+
+        Using very large Key or Value in HashMap potentially bloats memory since HashMap pre-allocates large size of
+        memory ((sizeof(Key) + sizeof(Value)) * N) for its backing storage's array. Using std::unique_ptr<> for JSRunLoopTimer's
+        PerVMData to keep HashMap's backing store size small.
+
+        * runtime/JSRunLoopTimer.cpp:
+        (JSC::JSRunLoopTimer::Manager::timerDidFire):
+        (JSC::JSRunLoopTimer::Manager::registerVM):
+        (JSC::JSRunLoopTimer::Manager::scheduleTimer):
+        (JSC::JSRunLoopTimer::Manager::cancelTimer):
+        (JSC::JSRunLoopTimer::Manager::timeUntilFire):
+        (JSC::JSRunLoopTimer::Manager::didChangeRunLoop):
+        * runtime/JSRunLoopTimer.h:
+
 2019-04-01  Stephan Szabo  <stephan.szabo@sony.com>
 

trunk/Source/JavaScriptCore/runtime/JSRunLoopTimer.cpp

@@ -122 +122 @@
         EpochTime nowEpochTime = epochTime(0_s);
         for (auto& entry : m_mapping) {
-            PerVMData& data = entry.value;
+            PerVMData& data = *entry.value;
 #if USE(CF)
             if (data.runLoop.get() != currentRunLoop)
@@ -173 +173 @@
 void JSRunLoopTimer::Manager::registerVM(VM& vm)
 {
-    PerVMData data { *this };
-#if USE(CF)
-    data.setRunLoop(this, vm.runLoop());
+    auto data = std::make_unique<PerVMData>(*this);
+#if USE(CF)
+    data->setRunLoop(this, vm.runLoop());
 #endif
 
@@ -200 +200 @@
     RELEASE_ASSERT(iter != m_mapping.end()); // We don't allow calling this after the VM dies.
 
-    PerVMData& data = iter->value;
+    PerVMData& data = *iter->value;
     EpochTime scheduleTime = fireEpochTime;
     bool found = false;
@@ -230 +230 @@
     }
 
-    PerVMData& data = iter->value;
+    PerVMData& data = *iter->value;
     EpochTime scheduleTime = epochTime(s_decade);
     for (unsigned i = 0; i < data.timers.size(); ++i) {
@@ -262 +262 @@
     RELEASE_ASSERT(iter != m_mapping.end()); // We only allow this to be called with a live VM.
 
-    PerVMData& data = iter->value;
+    PerVMData& data = *iter->value;
     for (auto& entry : data.timers) {
         if (entry.first.ptr() == &timer) {
@@ -280 +280 @@
     RELEASE_ASSERT(iter != m_mapping.end());
 
-    PerVMData& data = iter->value;
+    PerVMData& data = *iter->value;
     data.setRunLoop(this, newRunLoop);
 }

trunk/Source/JavaScriptCore/runtime/JSRunLoopTimer.h

@@ -79 +79 @@
 
         struct PerVMData {
-            PerVMData() = default;
 #if USE(CF)
             PerVMData(Manager&) { }
@@ -85 +84 @@
             PerVMData(Manager&);
 #endif
-            PerVMData(PerVMData&&) = default;
-            PerVMData& operator=(PerVMData&&) = default;
-
             ~PerVMData();
 
@@ -102 +98 @@
         };
 
-        HashMap<Ref<JSLock>, PerVMData> m_mapping;
+        HashMap<Ref<JSLock>, std::unique_ptr<PerVMData>> m_mapping;
     };