REGRESSION (r239814): Most classes that user Timer have 7 bytes of padding after...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Mar 2019 23:12:12 +0000 (23:12 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Mar 2019 23:12:12 +0000 (23:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194196

Reviewed by Simon Fraser.

Use std::nan as the value of m_unalignedNextFireTime to indicate the timer had been deleted
instead of having a dedicated boolean, which consumes 7 extra bytes for padding.

Note that some code in WebKit uses +Infinity as a fire time so we can't use that.

* platform/Timer.cpp:
(WebCore::TimerBase::TimerBase):
(WebCore::TimerBase::~TimerBase):
(WebCore::TimerBase::setNextFireTime):
(WebCore::TimerBase::nextUnalignedFireInterval const):
* platform/Timer.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243022 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/Timer.cpp
Source/WebCore/platform/Timer.h

index 932358f..a40e0f7 100644 (file)
@@ -1,3 +1,22 @@
+2019-03-15  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION (r239814): Most classes that user Timer have 7 bytes of padding after the Timer
+        https://bugs.webkit.org/show_bug.cgi?id=194196
+
+        Reviewed by Simon Fraser.
+
+        Use std::nan as the value of m_unalignedNextFireTime to indicate the timer had been deleted
+        instead of having a dedicated boolean, which consumes 7 extra bytes for padding.
+
+        Note that some code in WebKit uses +Infinity as a fire time so we can't use that.
+
+        * platform/Timer.cpp:
+        (WebCore::TimerBase::TimerBase):
+        (WebCore::TimerBase::~TimerBase):
+        (WebCore::TimerBase::setNextFireTime):
+        (WebCore::TimerBase::nextUnalignedFireInterval const):
+        * platform/Timer.h:
+
 2019-03-15  Sihui Liu  <sihui_liu@apple.com>
 
         [ Mojave WK1 ] Layout Test storage/indexeddb/database-odd-names.html is failing
index fd82402..1c36d53 100644 (file)
@@ -254,7 +254,6 @@ static bool shouldSuppressThreadSafetyCheck()
 }
 
 TimerBase::TimerBase()
-    : m_wasDeleted(false)
 {
 }
 
@@ -264,11 +263,9 @@ TimerBase::~TimerBase()
     RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get()) || shouldSuppressThreadSafetyCheck());
     stop();
     ASSERT(!inHeap());
-    if (m_heapItem) {
+    if (m_heapItem)
         m_heapItem->clearTimer();
-        m_heapItem = nullptr;
-    }
-    m_wasDeleted = true;
+    m_unalignedNextFireTime = MonotonicTime::nan();
 }
 
 void TimerBase::start(Seconds nextFireInterval, Seconds repeatInterval)
@@ -467,10 +464,13 @@ void TimerBase::setNextFireTime(MonotonicTime newTime)
 {
     ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
     RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get()) || shouldSuppressThreadSafetyCheck());
-    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!m_wasDeleted);
+    bool timerHasBeenDeleted = std::isnan(m_unalignedNextFireTime);
+    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!timerHasBeenDeleted);
 
-    if (m_unalignedNextFireTime != newTime)
+    if (m_unalignedNextFireTime != newTime) {
+        RELEASE_ASSERT(!std::isnan(newTime));
         m_unalignedNextFireTime = newTime;
+    }
 
     // Keep heap valid while changing the next-fire time.
     MonotonicTime oldTime = nextFireTime();
@@ -517,7 +517,9 @@ void TimerBase::didChangeAlignmentInterval()
 Seconds TimerBase::nextUnalignedFireInterval() const
 {
     ASSERT(isActive());
-    return std::max(m_unalignedNextFireTime - MonotonicTime::now(), 0_s);
+    auto result = std::max(m_unalignedNextFireTime - MonotonicTime::now(), 0_s);
+    RELEASE_ASSERT(std::isfinite(result));
+    return result;
 }
 
 } // namespace WebCore
index 6601675..17ef021 100644 (file)
@@ -96,7 +96,6 @@ private:
 
     MonotonicTime m_unalignedNextFireTime; // m_nextFireTime not considering alignment interval
     Seconds m_repeatInterval; // 0 if not repeating
-    bool m_wasDeleted { false };
 
     RefPtr<ThreadTimerHeapItem> m_heapItem;
     Ref<Thread> m_thread { Thread::current() };