[Webkit-unassigned] [Bug 168363] New: [GLib] GCActivityCallback::scheduleTimer() keeps pushing dispatch into the future

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=168363

            Bug ID: 168363
           Summary: [GLib] GCActivityCallback::scheduleTimer() keeps
                    pushing dispatch into the future
    Classification: Unclassified
           Product: WebKit
           Version: WebKit Nightly Build
          Hardware: Unspecified
                OS: Unspecified
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: JavaScriptCore
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: zan at falconsigh.net
                CC: bugs-noreply at webkitgtk.org, cgarcia at igalia.com,
                    mcatanzaro at igalia.com

GCActivityCallback::scheduleTimer() accepts a delay argument and, in case it's at most half the amount of the current delay, uses this new value to reschedule the internal timer.
https://trac.webkit.org/browser/trunk/Source/JavaScriptCore/heap/GCActivityCallback.cpp#L119

Say the GCActivityCallback is scheduled for the first time in 8 seconds. The timer is then set to dispatch 8 seconds from the current time (marked as CT#0). After the JS heap continues to grow, additional scheduleTimer() calls are done, each time decreasing the delay proportional to the amount of memory that's been allocated so far. This is calculated in GCActivityCallback::didAllocate().
https://trac.webkit.org/browser/trunk/Source/JavaScriptCore/heap/GCActivityCallback.cpp#L150

It's possible that the allocations progress in such way that, for example, after CT#0 + 7 seconds the computed delay drops to 4 seconds. At that point the call to scheduleTimer() reschedules the timer, but using the current time as the base, CT#1, which equals to CT#0 + 7, and uses 4 seconds as the delay amount, which means the next dispatch will be at CT#1 + 4 = CT#0 + 11 seconds, three seconds after the time when the first scheduled delay was supposed to dispatch.

This isn't how the USE(CF) implementation works. When the delay is set there for the first time, it's set as the latest time when the callback should be fired. Repeating the previous scenario, if the timer is first scheduled to be dispatched after 8 seconds from the current time (CT#0 + 8), and after 7 seconds the delay is dropped to 4 seconds, the new dispatch time will be CT#0 + 4, which is 3 seconds in the past, meaning the timer will dispatch ASAP.

The current GLib implementation should probably match that. Note that the current delay computation doesn't mean that GC doesn't get triggered at all, as far as I've tested it just means that the dispatch gets delayed long enough for the delay values to become really small and close enough to the present that the timer gets dispatched in the near-future main loop iteration.

The delays here are used to showcase the problem. The initial delays are usually quite large, but then become smaller as the allocations grow.

The USE(CF) implementation still sets m_nextFireTime to currentTime() + delay, just like we do, but note that there appears to be no use of the m_nextFireTime member variable or the GCActivityCallback::nextFireTime() getter.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170215/49a1d93b/attachment.html>