Bug 16537 – Make date code faster by removing redundant calls

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Eric Seidel (no email)]

Make date code faster by removing redundant calls

This is just one step in the right direction for the date code.  We're still executing way more than we need to each run through msToGregorianDateTime (which is called for basically every date function, since we store the date in ms since 1970 and have to convert to a GregorianDateTime struct each time before we can do anything interesting with the data).

[Eric Seidel (no email)]

Created attachment 18007 [details]
Make msFrom1970ToYear human-readable

 JavaScriptCore/kjs/DateMath.cpp |   30 ++++++++++++++++++------------
 1 files changed, 18 insertions(+), 12 deletions(-)

[Eric Seidel (no email)]

Created attachment 18008 [details]
Make msToDayInMonth slightly more readable and avoid recalculating msSince1970ToAbsoluteYear

 JavaScriptCore/kjs/DateMath.cpp |   90 +++++++++++++++++++--------------------
 1 files changed, 44 insertions(+), 46 deletions(-)

[Eric Seidel (no email)]

Created attachment 18009 [details]
Remove use of isInLeapYear to avoid calling msSince1970ToAbsoluteYear

 JavaScriptCore/kjs/DateMath.cpp |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

[Eric Seidel (no email)]

Created attachment 18010 [details]
Remove dayInYear call by changing msToDayInMonth to dayInMonthFromDayInYear

 JavaScriptCore/kjs/DateMath.cpp |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

[Eric Seidel (no email)]

Created attachment 18011 [details]
Remove more duplicate calls to dayInYear and getUTCOffset for further speedup

 JavaScriptCore/kjs/DateMath.cpp |   40 +++++++++++++++++++++-----------------
 1 files changed, 22 insertions(+), 18 deletions(-)

[Geoffrey Garen]

Comment on attachment 18007 [details]
Make msFrom1970ToYear human-readable

r=me

[Geoffrey Garen]

Comment on attachment 18008 [details]
Make msToDayInMonth slightly more readable and avoid recalculating msSince1970ToAbsoluteYear

-static inline int msToYear(double ms)
+static inline int msSince1970ToAbsoluteYear(double msSince1970)

I don't really agree that this change is cleanup.

In the date code, "ms" always means "msSince1970." Substituting "msSince1970" makes the code read awkwardly for me. Also, I think it's a mistake to put "msSince1970" in only select places -- that makes it seem as if there's a difference between "ms" functions and "msSince1970" functions, when there's not.

Perhaps it would be better just to add a comment at the top of the file stating that "ms" means "ms since 1970."

I'm also not sure what you mean by the term "AbsoluteYear." What is an absolute year? (/me imagines an ad for Absolut Vodka on the back of the New Yorker.)

+    static int minYear = std::min(msSince1970ToAbsoluteYear(getCurrentUTCTime()), maximumYearForDST()-27) ;

Need a space around the "-" here.

The checkMonth change looks good. I wish the function had a name that hinted that it changed it arguments, but I can't think of a good one.

Not sure what to do with this patch. I guess I'll r- for the comments above.

[Geoffrey Garen]

Comment on attachment 18009 [details]
Remove use of isInLeapYear to avoid calling msSince1970ToAbsoluteYear

r=me

[Geoffrey Garen]

Comment on attachment 18010 [details]
Remove dayInYear call by changing msToDayInMonth to dayInMonthFromDayInYear

r=me

[Geoffrey Garen]

Comment on attachment 18011 [details]
Remove more duplicate calls to dayInYear and getUTCOffset for further speedup

r=me

[Geoffrey Garen]

I think you should run the JavaScriptCore tests with your machine set to interesting locales (on both sides of the international date line) before landing these patches.

[Eric Seidel (no email)]

Created attachment 18012 [details]
Final combined patch w/ ggaren's suggested fixes


        Reviewed by Geoff.

        Small reworking of Date code for 4% speedup on Date tests (0.2% overall)
        http://bugs.webkit.org/show_bug.cgi?id=16537

        Make msToYear human-readable
        Make msToDayInMonth slightly more readable and avoid recalculating msToYear
        Remove use of isInLeapYear to avoid calling msToYear
        Remove dayInYear call by changing msToDayInMonth to dayInMonthFromDayInYear
        Remove more duplicate calls to dayInYear and getUTCOffset for further speedup

        * kjs/DateMath.cpp:
        (KJS::daysFrom1970ToYear):
        (KJS::msToYear):
        (KJS::monthFromDayInYear):
        (KJS::checkMonth):
        (KJS::dayInMonthFromDayInYear):
        (KJS::dateToDayInYear):
        (KJS::getDSTOffsetSimple):
        (KJS::getDSTOffset):
        (KJS::gregorianDateTimeToMS):
        (KJS::msToGregorianDateTime):
---
 JavaScriptCore/ChangeLog        |   25 +++++++
 JavaScriptCore/kjs/DateMath.cpp |  146 ++++++++++++++++++++-------------------
 2 files changed, 99 insertions(+), 72 deletions(-)

[David Kilzer (:ddkilzer)]

Fixed in r28913.