Possible performance enhancements for CalendarUtil.getDaysBetween()

65 views
Skip to first unread message

Davide Cavestro

unread,
Jan 30, 2015, 10:42:42 AM1/30/15
to google-we...@googlegroups.com
I guess com.google.gwt.user.datepicker.client.CalendarUtil.getDaysBetween(Date, Date) was written with datepicker in mind, BTW I've seen some code making heavy use of that method, and its performance was badly impacted.
Since actual implementation internally instantiates two new dates per every invocation and then resets the time for each of them, I've simply rewritten the same logic avoiding date instantiation and time resets, and this gave me good results.
So I hope CalendarUtil.getDaysBetween() could be enhanced the same way.

Follows an excerpt of a naive test comparing one shot original vs optimized implementation... clearly this kind of measurement doesn't consider GC time (mainly induced by original implementation) so I'd consider its results pessimistic. Nonetheless running it with Chrome 37 on my host, the execution time of optimized code is almost constantly 25% of original one.

public class Main implements EntryPoint {

    @Override
    public void onModuleLoad() {
        final int count = 100000;
        
        final Date prev = new Date(0);
        final Date next = new Date(0);
        long optTime = 0;
        long origTime = 0;
        
        for (int i = 0; i<count;i++) {
            next.setTime(prev.getTime()+1000*60*60*24);
            
            final long startOrig = System.currentTimeMillis ();
            final int origResult = CalendarUtil.getDaysBetween (prev, next);
            origTime+= System.currentTimeMillis ()-startOrig;
            
            final long startOpt = System.currentTimeMillis ();
            final int optResult = getDaysBetween (prev, next);
            optTime+= System.currentTimeMillis ()-startOpt;
            
            prev.setTime(next.getTime());
            if (optResult!=origResult) {
                throw new RuntimeException ("Ouch");
            }
        }
        Window.alert(count+" steps took \noptimized code: "+optTime+"\noriginal code: "+origTime+"ms");

//        GWT.log ("Optimized implementation> "+count+" steps took "+optTime+"ms");
//        GWT.log ("Original implementation> "+count+" steps took "+origTime+"ms");
    }
    
    /**
     * {@code CalendarUtil.getDaysBetween()} optimized reimplementation.
     * 
     * @see CalendarUtil#getDaysBetween(Date, Date)
     */
    protected int getDaysBetween(final Date start, final Date finish) {
        // Extracts midnight time for both dates
        final long aTime = (start.getTime() / 1000) * 1000;
        final long bTime = (finish.getTime() / 1000) * 1000;

        long adjust = 60 * 60 * 1000;
        adjust = (bTime > aTime) ? adjust : -adjust;

        return (int) ((bTime - aTime + adjust) / (24 * 60 * 60 * 1000));
    }
}

Talking about real world code instead, this change practically removed getDaysBetween effects from CPU profiling charts, while it was originally one of the main components.

Jens

unread,
Jan 30, 2015, 9:08:15 PM1/30/15
to google-we...@googlegroups.com
Your implementation does not have same behavior as the original GWT one.

GWTs implementation resets hours, minutes, seconds and milliseconds while your implementation only resets milliseconds AND you reset them wrong as well :) For negative getTime() your calculation moves time to the next second into the future which is wrong because when you reset time you always want to go into the past (so you do not modify the actual day accidentally)

As you only have modified the way aTime and bTime are calculated I have printed them for current GWT implementation and your new implementation. See output:

start: 1969-12-31 22:30:30.500
end: 1970-01-02 15:30:30.500

current GWT aTime: -90000000 => Date: 1969-12-31 00:00:00.000
current GWT bTime: 82800000 => Date: 1970-01-02 00:00:00.000
current GWT days between: 2

new aTime: -8969000 => Date: 1969-12-31 22:30:31.000
new bTime: 138630000 => Date: 1970-01-02 15:30:30.000
new days between: 1

As you can see your implementation does not reset the full time and your millisecond reset moves the start date 500ms into the future. 

GWT had the same millisecond reset bug until I fixed it in https://gwt-review.googlesource.com/#/c/7462/3 
This bug has caused some exceptions in DatePicker for dates that are within 1000ms before epoch as described in the corresponding bug report: https://code.google.com/p/google-web-toolkit/issues/detail?id=8653


-- J.

Davide Cavestro

unread,
Jan 31, 2015, 9:28:10 AM1/31/15
to google-we...@googlegroups.com
I must admit I didn't spend enough time to fully understand why the gwt original  implementation did all that stuff for resetting time. Many thanks for the detailed explanation.
At the moment I have no access to dev tools, anyway I'm just wondering if there's still room for optimizations without incurring in bugs: i.e. if resetting dates time is needed, then CalendarUtils could keep two private static Date instances instead of instantiating two new dates per call (that should be safe until there's no multithreading)

--
You received this message because you are subscribed to a topic in the Google Groups "Google Web Toolkit" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/google-web-toolkit/OVnGQi0k-Y4/unsubscribe.
To unsubscribe from this group and all its topics, send an email to google-web-tool...@googlegroups.com.
To post to this group, send email to google-we...@googlegroups.com.
Visit this group at http://groups.google.com/group/google-web-toolkit.
For more options, visit https://groups.google.com/d/optout.

Reply all
Reply to author
Forward
0 new messages