Skip to content

Commit

Permalink
[WTF] Avoid MonotonicTime::now / WallTime::now calls if relative seco…
Browse files Browse the repository at this point in the history
…nds are infinity

https://bugs.webkit.org/show_bug.cgi?id=256143
rdar://108706987

Reviewed by Mark Lam.

This is micro-optimization. If relative seconds from now is infinity, we should not call MonotonicTime::now() / WallTime::now()
to compute the result. They involve system calls, and it is common that relative seconds are infinity (for example, just waiting
Condition without timeout is infinity).

* Source/WTF/wtf/ApproximateTime.cpp:
(WTF::ApproximateTime::approximateWallTime const):
(WTF::ApproximateTime::approximateMonotonicTime const):
* Source/WTF/wtf/Condition.h:
* Source/WTF/wtf/GenericTimeMixin.h:
(WTF::GenericTimeMixin::timePointFromNow):
* Source/WTF/wtf/MessageQueue.h:
(WTF::MessageQueue<DataType>::waitForMessageFilteredWithTimeout):
* Source/WTF/wtf/MonotonicTime.cpp:
(WTF::MonotonicTime::approximateWallTime const):
* Source/WTF/wtf/ParkingLot.cpp:
(WTF::ParkingLot::parkConditionallyImpl):
* Source/WTF/wtf/WTFSemaphore.h:
* Source/WTF/wtf/WallTime.cpp:
(WTF::WallTime::approximateMonotonicTime const):
* Source/WTF/wtf/threads/BinarySemaphore.h:
* Source/WTF/wtf/win/RunLoopWin.cpp:
(WTF::RunLoop::TimerBase::timerFired):
(WTF::RunLoop::TimerBase::start):

Canonical link: https://commits.webkit.org/263538@main
  • Loading branch information
Constellation committed Apr 30, 2023
1 parent b6e0a72 commit ab0f8a1
Show file tree
Hide file tree
Showing 15 changed files with 42 additions and 15 deletions.
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/runtime/WaiterListManager.cpp
Expand Up @@ -55,7 +55,7 @@ WaiterListManager::WaitSyncResult WaiterListManager::waitSyncImpl(VM& vm, ValueT
{
Ref<Waiter> syncWaiter = vm.syncWaiter();
Ref<WaiterList> list = findOrCreateList(ptr);
MonotonicTime time = MonotonicTime::now() + timeout;
MonotonicTime time = MonotonicTime::timePointFromNow(timeout);

{
Locker listLocker { list->lock };
Expand Down
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/runtime/Watchdog.cpp
Expand Up @@ -70,7 +70,7 @@ bool Watchdog::shouldTerminate(JSGlobalObject* globalObject)
// but it is a fact that only Windows is experiencing the issue.
epsilon = Seconds::fromMilliseconds(20);
#endif
if (MonotonicTime::now() + epsilon < m_deadline)
if (MonotonicTime::timePointFromNow(epsilon) < m_deadline)
return false; // Just a stale timer firing. Nothing to do.

// Set m_deadline to MonotonicTime::infinity() here so that we can reject all future
Expand Down
4 changes: 4 additions & 0 deletions Source/WTF/wtf/ApproximateTime.cpp
Expand Up @@ -34,11 +34,15 @@ namespace WTF {

WallTime ApproximateTime::approximateWallTime() const
{
if (std::isinf(*this))
return WallTime::fromRawSeconds(m_value);
return *this - now() + WallTime::now();
}

MonotonicTime ApproximateTime::approximateMonotonicTime() const
{
if (std::isinf(*this))
return MonotonicTime::fromRawSeconds(m_value);
return *this - now() + MonotonicTime::now();
}

Expand Down
8 changes: 4 additions & 4 deletions Source/WTF/wtf/Condition.h
Expand Up @@ -96,24 +96,24 @@ class Condition final {
template<typename LockType, typename Functor>
bool waitFor(LockType& lock, Seconds relativeTimeout, const Functor& predicate)
{
return waitUntil(lock, MonotonicTime::now() + relativeTimeout, predicate);
return waitUntil(lock, MonotonicTime::timePointFromNow(relativeTimeout), predicate);
}

template<typename Functor>
bool waitFor(Lock& lock, Seconds relativeTimeout, const Functor& predicate) WTF_REQUIRES_LOCK(lock)
{
return waitUntil(lock, MonotonicTime::now() + relativeTimeout, predicate);
return waitUntil(lock, MonotonicTime::timePointFromNow(relativeTimeout), predicate);
}

template<typename LockType>
bool waitFor(LockType& lock, Seconds relativeTimeout)
{
return waitUntil(lock, MonotonicTime::now() + relativeTimeout);
return waitUntil(lock, MonotonicTime::timePointFromNow(relativeTimeout));
}

bool waitFor(Lock& lock, Seconds relativeTimeout) WTF_REQUIRES_LOCK(lock)
{
return waitUntil(lock, MonotonicTime::now() + relativeTimeout);
return waitUntil(lock, MonotonicTime::timePointFromNow(relativeTimeout));
}

template<typename LockType>
Expand Down
7 changes: 7 additions & 0 deletions Source/WTF/wtf/GenericTimeMixin.h
Expand Up @@ -119,6 +119,13 @@ class GenericTimeMixin {
return *static_cast<const DerivedTime*>(this);
}

static constexpr DerivedTime timePointFromNow(Seconds relativeTimeFromNow)
{
if (std::isinf(relativeTimeFromNow))
return DerivedTime::fromRawSeconds(relativeTimeFromNow.value());
return DerivedTime::now() + relativeTimeFromNow;
}

protected:
// This is the epoch. So, x.secondsSinceEpoch() should be the same as x - DerivedTime().
constexpr GenericTimeMixin() = default;
Expand Down
2 changes: 1 addition & 1 deletion Source/WTF/wtf/MessageQueue.h
Expand Up @@ -141,7 +141,7 @@ namespace WTF {
Locker lock { m_lock };
bool timedOut = false;

MonotonicTime absoluteTimeout = MonotonicTime::now() + relativeTimeout;
MonotonicTime absoluteTimeout = MonotonicTime::timePointFromNow(relativeTimeout);
auto found = m_queue.end();
while (!m_killed && !timedOut) {
found = m_queue.findIf([&predicate](const std::unique_ptr<DataType>& ptr) -> bool {
Expand Down
2 changes: 2 additions & 0 deletions Source/WTF/wtf/MonotonicTime.cpp
Expand Up @@ -33,6 +33,8 @@ namespace WTF {

WallTime MonotonicTime::approximateWallTime() const
{
if (std::isinf(*this))
return WallTime::fromRawSeconds(m_value);
return *this - now() + WallTime::now();
}

Expand Down
5 changes: 2 additions & 3 deletions Source/WTF/wtf/ParkingLot.cpp
Expand Up @@ -592,9 +592,8 @@ NEVER_INLINE ParkingLot::ParkResult ParkingLot::parkConditionallyImpl(
{
MutexLocker locker(me->parkingLock);
while (me->address && timeout.nowWithSameClock() < timeout) {
me->parkingCondition.timedWait(
me->parkingLock, timeout.approximateWallTime());

me->parkingCondition.timedWait(me->parkingLock, timeout.approximateWallTime());

// It's possible for the OS to decide not to wait. If it does that then it will also
// decide not to release the lock. If there's a bug in the time math, then this could
// result in a deadlock. Flashing the lock means that at worst it's just a CPU-eating
Expand Down
2 changes: 1 addition & 1 deletion Source/WTF/wtf/WTFSemaphore.h
Expand Up @@ -61,7 +61,7 @@ class Semaphore final {

bool waitFor(Seconds relativeTimeout)
{
return waitUntil(MonotonicTime::now() + relativeTimeout);
return waitUntil(MonotonicTime::timePointFromNow(relativeTimeout));
}

void wait()
Expand Down
2 changes: 2 additions & 0 deletions Source/WTF/wtf/WallTime.cpp
Expand Up @@ -33,6 +33,8 @@ namespace WTF {

MonotonicTime WallTime::approximateMonotonicTime() const
{
if (std::isinf(*this))
return MonotonicTime::fromRawSeconds(m_value);
return *this - now() + MonotonicTime::now();
}

Expand Down
7 changes: 7 additions & 0 deletions Source/WTF/wtf/posix/ThreadingPOSIX.cpp
Expand Up @@ -606,6 +606,13 @@ void ThreadCondition::wait(Mutex& mutex)

bool ThreadCondition::timedWait(Mutex& mutex, WallTime absoluteTime)
{
if (std::isinf(absoluteTime)) {
if (absoluteTime == -WallTime::infinity())
return false;
wait(mutex);
return true;
}

if (absoluteTime < WallTime::now())
return false;

Expand Down
2 changes: 1 addition & 1 deletion Source/WTF/wtf/threads/BinarySemaphore.h
Expand Up @@ -43,7 +43,7 @@ class BinarySemaphore final {

bool waitFor(Seconds relativeTimeout)
{
return waitUntil(MonotonicTime::now() + relativeTimeout);
return waitUntil(MonotonicTime::timePointFromNow(relativeTimeout));
}

void wait()
Expand Down
4 changes: 2 additions & 2 deletions Source/WTF/wtf/win/RunLoopWin.cpp
Expand Up @@ -147,7 +147,7 @@ void RunLoop::TimerBase::timerFired()
m_isActive = false;
::KillTimer(m_runLoop->m_runLoopMessageWindow, bitwise_cast<uintptr_t>(this));
} else
m_nextFireDate = MonotonicTime::now() + m_interval;
m_nextFireDate = MonotonicTime::timePointFromNow(m_interval);
}

fired();
Expand All @@ -169,7 +169,7 @@ void RunLoop::TimerBase::start(Seconds interval, bool repeat)
m_isRepeating = repeat;
m_isActive = true;
m_interval = interval;
m_nextFireDate = MonotonicTime::now() + m_interval;
m_nextFireDate = MonotonicTime::timePointFromNow(m_interval);
::SetTimer(m_runLoop->m_runLoopMessageWindow, bitwise_cast<uintptr_t>(this), interval.millisecondsAs<UINT>(), nullptr);
}

Expand Down
6 changes: 6 additions & 0 deletions Source/WTF/wtf/win/ThreadingWin.cpp
Expand Up @@ -351,6 +351,12 @@ void Mutex::unlock()
// Returns an interval in milliseconds suitable for passing to one of the Win32 wait functions (e.g., ::WaitForSingleObject).
static DWORD absoluteTimeToWaitTimeoutInterval(WallTime absoluteTime)
{
if (std::isinf(absoluteTime)) {
if (absoluteTime == -WallTime::infinity())
return 0;
return INFINITE;
}

WallTime currentTime = WallTime::now();

// Time is in the past - return immediately.
Expand Down
Expand Up @@ -59,7 +59,7 @@
bool waitFor(Seconds timeout) final
{
Locker locker { m_lock };
auto absoluteTime = MonotonicTime::now() + timeout;
auto absoluteTime = MonotonicTime::timePointFromNow(timeout);
return m_condition.waitUntil(m_lock, absoluteTime, [&] {
assertIsHeld(m_lock);
return m_isSet;
Expand Down

0 comments on commit ab0f8a1

Please sign in to comment.