Summary: | Make date code faster by removing redundant calls | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||||||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | ||||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Mac | ||||||||||||||||
OS: | OS X 10.4 | ||||||||||||||||
Attachments: |
|
Description
Eric Seidel (no email)
2007-12-20 10:42:33 PST
Created attachment 18007 [details]
Make msFrom1970ToYear human-readable
JavaScriptCore/kjs/DateMath.cpp | 30 ++++++++++++++++++------------
1 files changed, 18 insertions(+), 12 deletions(-)
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(-)
Created attachment 18009 [details]
Remove use of isInLeapYear to avoid calling msSince1970ToAbsoluteYear
JavaScriptCore/kjs/DateMath.cpp | 9 ++-------
1 files changed, 2 insertions(+), 7 deletions(-)
Created attachment 18010 [details]
Remove dayInYear call by changing msToDayInMonth to dayInMonthFromDayInYear
JavaScriptCore/kjs/DateMath.cpp | 17 ++++++++---------
1 files changed, 8 insertions(+), 9 deletions(-)
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(-)
Comment on attachment 18007 [details]
Make msFrom1970ToYear human-readable
r=me
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.
Comment on attachment 18009 [details]
Remove use of isInLeapYear to avoid calling msSince1970ToAbsoluteYear
r=me
Comment on attachment 18010 [details]
Remove dayInYear call by changing msToDayInMonth to dayInMonthFromDayInYear
r=me
Comment on attachment 18011 [details]
Remove more duplicate calls to dayInYear and getUTCOffset for further speedup
r=me
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. 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(-) Fixed in r28913. |