Bug 15853 – date-format-xparb spends 5% of TOTAL TIME in notify_check

• Component: JavaScriptCore
• Status: RESOLVED DUPLICATE

[Eric Seidel (no email)]

date-format-xparb spends 5% of TOTAL TIME in notify_check

We should move away from using notify_check and instead register for a UNIX signal for when the notify happens.  That would keep us from needing to poll.

[Darin Adler]

Some of this might be addressed by the patch I attached to bug 15852.

[Geoffrey Garen]

I'm not sure there is such a Unix signal. Unix signals are also a pretty bad way to do IPC. They can happen at any time, and very few operations are safe within a signal handler.

Maybe a better solution would be to check only every so often, instead of all the time. After all, we're checking for a rare event.

[Eric Seidel (no email)]

Well, regarding the unix signal: you get to pick your own.  Check out the API in /usr/lib/notify.h

As mjs and I discussed you would sign up for the signal and then have a global var cache of the timezone and two global var counters.  Assuming ++ is atomic on all platforms we care about, then you just have your sig handler increment one of the counters.  Every time we go to access the timezone info, we first check if the counters match.  If they don't then we assign the other counter to the signal counter's value, and then go and update the cached timezone info (using a normal localtime_r call, or whatever.

That should be thread safe.  All your doing is incrementing a global counter in a sig-handler, so it shouldn't matter what thread it's delivered on.  If another signal is delivered mid update, at any point during the update, the worst it should cause is that you end up updating with the same timezone twice.  Never should you miss a timezone update.

This method should be much faster than notify_check (which for whatever reason is ridiculously slow.  It uses a shared memory region and probably some locking of some form).  localtime_r also uses a mutex and notify_check, sadly.

[Eric Seidel (no email)]

The code looks something like this:

void timezoneChangedSigHandler()
{
     gTimeZoneNotifies++;
}

void doSomethingNeedingTheTimeZone()
{
    // current code would notify_check, instead we...
    if (gTimeZoneNotifies != gLastCachedNotify) {
        gLastCachedNotify = gTimeZoneNotifies;
        gTimeZone = // do the localtime_r dance to get the timezone
    }
    // use the timezone
}

I think that's safe... but it's also 11pm and I got a whole 2 hours of sleep last night or something... so it's possible I missed something obvious.

[Alexey Proskuryakov]

Eric, is this bug still valid?

[Eric Seidel (no email)]

I expect this bug does still have some validity.  We'd have to see how slow our date code is these days.  Someone would have to look at the output from:
run-sunspider --shark date-format-xparb

[Geoffrey Garen]

Reading this now, I like Eric's last idea, but this is probably obsoleted by bug 31197.

*** This bug has been marked as a duplicate of bug 31197 ***