Improve Date Performance (patch supplied)

[jimbogithub]

GWT version: 2.8 Browser (with version): Any Operating System: Any

---

Description

Unnecessary long conversion is impacting Date performance, especially methods like compareTo() which are heavily used in sorted collections etc. This can be improved by simply delegating to the JsDate double.

A proposed patch is provided below. Hoping a committer can take this on so that I don't have to overcome the initial Contributor hurdles for this small change.

Proposed Patch

From 8bbc05c26d56534498d9d9af013f6228c17b9251 Mon Sep 17 00:00:00 2001
From: James Olsen
Date: Sat, 11 Mar 2017 18:07:47 +1300
Subject: [PATCH] Avoid Long conversion in emulated java.util.Date where
 possible to improve performance.  Real world performance is significantly improved.

---
 user/super/com/google/gwt/emul/java/util/Date.java | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/user/super/com/google/gwt/emul/java/util/Date.java b/user/super/com/google/gwt/emul/java/util/Date.java
index fe19568..adc471a 100644
--- a/user/super/com/google/gwt/emul/java/util/Date.java
+++ b/user/super/com/google/gwt/emul/java/util/Date.java
@@ -38,11 +38,15 @@ public class Date implements Cloneable, Comparable<Date>, Serializable {
   }

   public static long parse(String s) {
+    return (long) parseDbl(s);
+  }
+
+  private static double parseDbl(String s) {
     double parsed = NativeDate.parse(s);
     if (Double.isNaN(parsed)) {
       throw new IllegalArgumentException();
     }
-    return (long) parsed;
+    return parsed;
   }

   // CHECKSTYLE_OFF: Matching the spec.
@@ -94,30 +98,34 @@ public class Date implements Cloneable, Comparable<Date>, Serializable {
     jsdate = new NativeDate(date);
   }

+  private Date(double date) {
+    jsdate = new NativeDate(date);
+  }
+
   public Date(String date) {
-    this(Date.parse(date));
+    this(Date.parseDbl(date));
   }

   public boolean after(Date when) {
-    return getTime() > when.getTime();
+    return jsdate.getTime() > when.jsdate.getTime();
   }

   public boolean before(Date when) {
-    return getTime() < when.getTime();
+    return jsdate.getTime() < when.jsdate.getTime();
   }

   public Object clone() {
-    return new Date(getTime());
+    return new Date(jsdate.getTime());
   }

   @Override
   public int compareTo(Date other) {
-    return Long.compare(getTime(), other.getTime());
+    return Double.compare(jsdate.getTime(), other.jsdate.getTime());
   }

   @Override
   public boolean equals(Object obj) {
-    return ((obj instanceof Date) && (getTime() == ((Date) obj).getTime()));
+    return ((obj instanceof Date) && (jsdate.getTime() == ((Date) obj).jsdate.getTime()));
   }

   public int getDate() {
-- 
2.8.1

From 645ae2af57d527b9271ad417a09e5b2679557541 Mon Sep 17 00:00:00 2001
From: James Olsen
Date: Sat, 11 Mar 2017 18:12:45 +1300
Subject: [PATCH] Conform to coding standards.

---
 user/super/com/google/gwt/emul/java/util/Date.java | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/user/super/com/google/gwt/emul/java/util/Date.java b/user/super/com/google/gwt/emul/java/util/Date.java
index adc471a..76a5a6c 100644
--- a/user/super/com/google/gwt/emul/java/util/Date.java
+++ b/user/super/com/google/gwt/emul/java/util/Date.java
@@ -98,14 +98,14 @@ public class Date implements Cloneable, Comparable<Date>, Serializable {
     jsdate = new NativeDate(date);
   }

-  private Date(double date) {
-    jsdate = new NativeDate(date);
-  }
-
   public Date(String date) {
     this(Date.parseDbl(date));
   }

+  private Date(double date) {
+    jsdate = new NativeDate(date);
+  }
+
   public boolean after(Date when) {
     return jsdate.getTime() > when.jsdate.getTime();
   }
-- 
2.8.1

[jimbogithub]

@dankurka, @gkdn, @nordligulv - I think there are some similar opportunities in the java.sql subclasses.

[niloc132]

Looks like an interesting idea, though I think that GWT 2.8 already effectively uses double when the "long" is small enough that it doesn't need extra precision.

Have you filed a CLA? Unfortunately, we can't use your code until you do. I understand that is one of the big hurdles to contributing, but we don't add it lightly - there are legal concerns to using someone's code without their express authorization, and the CLA is how this project manages those concerns.

[jimbogithub]

CLA is now signed.

GWT is definitely using emulated Longs in the compareTo() method for present date values. This is observable via the Chrome debugger. Changing to delegate to the native double gives me a 30% real world performance improvement when loading items into a date sorted collection. This must be a much greater relative improvement in the compareTo() itself.

[niloc132]

I'm not sure how to confirm a CLA from just a github name, perhaps one of the gerrit admins can help out. Normally you would just push the patch to gerrit (which doesn't even work without the CLA), and we review from there, or have someone else take over, etc. Once someone confirms it, I'll be happy to try to shepherd this forward.

[jnehlmeier]

AFAICT all long operations go though LongLib and LongLib then decides wether the long is represented by a native JS number or as an emulated long. Currently all longs that fit into 44 bits should be handled as native JS numbers unless there is a bug somewhere.

https://github.com/gwtproject/gwt/blob/master/dev/core/super/com/google/gwt/lang/LongLib.java

But it also means that Dates can be constructed with long values between 2^44 and 2^53 that are indeed slow emulated longs. While Java Long has a max values of 2^63 - 1 the JS specification has an lower/upper limit for JS dates which is roughly around +-2^53. There is an open bug that suggests letting Date emulation fail fast if a larger long is provided. Currently the Date simply becomes invalid.

A quick example shows that GWT 2.8 properly uses native JS numbers for present dates in its compare method. So are you sure you are using GWT 2.8? Can you share an example project that uses emulated slow longs for dates that have milliseconds within +-2^44 or does your app use dates outside that range?

[jimbogithub]

The following tests demonstrate that a Long representing current time in millis is slower than a Double doing same. Other variants are provided for further comparison.

import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.Date;
import java.util.logging.Level;
import java.util.logging.Logger;

import com.google.gwt.junit.client.GWTTestCase;

public class ComparatorGWTTest extends GWTTestCase {

    private final Logger log = Logger.getLogger(ComparatorGWTTest.class.getSimpleName());

    private static final int SIZE = 10000;

    public void testLongSmall() {
        log.log(Level.INFO, "testLongSmall " + doLong(1, null));
        log.log(Level.INFO, "testLongSmall " + doLong(1, null));
        log.log(Level.INFO, "testLongSmall " + doLong(1, null));
    }

    public void testLongBig() {
        log.log(Level.INFO, "testLongBig " + doLong(Long.MAX_VALUE - (SIZE * 2), null));
        log.log(Level.INFO, "testLongBig " + doLong(Long.MAX_VALUE - (SIZE * 2), null));
        log.log(Level.INFO, "testLongBig " + doLong(Long.MAX_VALUE - (SIZE * 2), null));
    }

    public void testLongCurrent() {
        log.log(Level.INFO, "testLongCurrent " + doLong(System.currentTimeMillis(), null));
        log.log(Level.INFO, "testLongCurrent " + doLong(System.currentTimeMillis(), null));
        log.log(Level.INFO, "testLongCurrent " + doLong(System.currentTimeMillis(), null));
    }

    public void testLongCurrentWithComparator() {
        log.log(Level.INFO,
                "testLongCurrentWithComparator " + doLong(System.currentTimeMillis(), new LongComparator()));
        log.log(Level.INFO,
                "testLongCurrentWithComparator " + doLong(System.currentTimeMillis(), new LongComparator()));
        log.log(Level.INFO,
                "testLongCurrentWithComparator " + doLong(System.currentTimeMillis(), new LongComparator()));
    }

    public void testDouble() {
        log.log(Level.INFO, "testDouble " + doDouble(System.currentTimeMillis(), null));
        log.log(Level.INFO, "testDouble " + doDouble(System.currentTimeMillis(), null));
        log.log(Level.INFO, "testDouble " + doDouble(System.currentTimeMillis(), null));
    }

    public void testDoubleWithComparator() {
        log.log(Level.INFO, "testDoubleWithComparator " + doDouble(System.currentTimeMillis(), new DoubleComparator()));
        log.log(Level.INFO, "testDoubleWithComparator " + doDouble(System.currentTimeMillis(), new DoubleComparator()));
        log.log(Level.INFO, "testDoubleWithComparator " + doDouble(System.currentTimeMillis(), new DoubleComparator()));
    }

    public void testDate() {
        log.log(Level.INFO, "testDate " + doDate(System.currentTimeMillis(), null));
        log.log(Level.INFO, "testDate " + doDate(System.currentTimeMillis(), null));
        log.log(Level.INFO, "testDate " + doDate(System.currentTimeMillis(), null));
    }

    public void testDateWithComparator() {
        log.log(Level.INFO, "testDateWithComparator " + doDate(System.currentTimeMillis(), new DateComparator()));
        log.log(Level.INFO, "testDateWithComparator " + doDate(System.currentTimeMillis(), new DateComparator()));
        log.log(Level.INFO, "testDateWithComparator " + doDate(System.currentTimeMillis(), new DateComparator()));
    }

    private long doLong(long seed, Comparator<Long> comparator) {
        ArrayList<Long> longs = new ArrayList<>(SIZE);
        long item = seed;

        long start = System.currentTimeMillis();

        for (int i = 0; i < SIZE; i++) {
            int index = Collections.binarySearch(longs, item);
            if (index >= 0) {
                longs.set(index, item);
            } else {
                int insertIndex = -index - 1;
                longs.add(insertIndex, item);
            }
            item++;
        }

        long end = System.currentTimeMillis();
        return end - start;
    }

    private long doDouble(double seed, Comparator<Double> comparator) {
        ArrayList<Double> doubles = new ArrayList<>(SIZE);
        double item = seed;

        long start = System.currentTimeMillis();

        for (int i = 0; i < SIZE; i++) {
            int index = Collections.binarySearch(doubles, item, comparator);
            if (index >= 0) {
                doubles.set(index, item);
            } else {
                int insertIndex = -index - 1;
                doubles.add(insertIndex, item);
            }
            item++;
        }

        long end = System.currentTimeMillis();
        return end - start;
    }

    private long doDate(long seed, Comparator<Date> comparator) {
        ArrayList<Date> doubles = new ArrayList<>(SIZE);
        Date item = new Date(seed);

        long start = System.currentTimeMillis();

        for (int i = 0; i < SIZE; i++) {
            int index = Collections.binarySearch(doubles, item, comparator);
            if (index >= 0) {
                doubles.set(index, item);
            } else {
                int insertIndex = -index - 1;
                doubles.add(insertIndex, item);
            }
            item = new Date(item.getTime() + 1);
        }

        long end = System.currentTimeMillis();
        return end - start;
    }

    @Override
    public String getModuleName() {
        return "your.module.Here";
    }

    class LongComparator implements Comparator<Long> {

        @Override
        public int compare(Long o1, Long o2) {
            return o1.compareTo(o2);
        }

    }

    class DoubleComparator implements Comparator<Double> {

        @Override
        public int compare(Double o1, Double o2) {
            return o1.compareTo(o2);
        }

    }

    class DateComparator implements Comparator<Date> {

        @Override
        public int compare(Date o1, Date o2) {
            return o1.compareTo(o2);
        }

    }

}

[jimbogithub]

Change-Id: I5550713256de1d4f332736f8d8bd410205e6a8e5 pushed to Gerrit.

Two com.google.gwt.i18n.I18NSuite tests failed for me but from a quick look I'm pretty sure that's because the tests themselves don't work when run in a TZ with local date ahead of assumed UTC date (I'm in +13 and was running in my A.M. so still prior UTC day). For the record they were:

Testcase: testChineseDateParse took 5.154 sec
    FAILED
expected: <23>, actual: <24>
junit.framework.AssertionFailedError: expected: <23>, actual: <24>
    at com.google.gwt.i18n.client.DateTimeParse_zh_CN_Test.testChineseDateParse(DateTimeParse_zh_CN_Test.java:72)

Testcase: testPredefDateFormat took 0.066 sec
    FAILED
expected: <Short: 2010-02-01>, actual: <Short: 2010-02-02>
junit.framework.AssertionFailedError: expected: <Short: 2010-02-01>, actual: <Short: 2010-02-02>
    at com.google.gwt.i18n.client.I18N2Test.testPredefDateFormat(I18N2Test.java:460)

I also spotted the following com.google.gwt.emultest.CollectionsSuite failure but I think it worked fine. The test expects an IllegalArgumentException which is being thrown but is still being reported as an ERROR.

Testcase: testParse took 0.099 sec
    Caused an ERROR
null
java.lang.RuntimeException(EXACT TYPE UNKNOWN)
    at java.lang.Throwable.Throwable(Throwable.java:61)
    at java.lang.RuntimeException.RuntimeException(Exception.java:25)
    at java.lang.IllegalArgumentException.IllegalArgumentException(IllegalArgumentException.java:25)
    at java.util.Date.parseDbl(Date.java:47)
    at com.google.gwt.emultest.java.util.DateTest.testParse(Date.java:41)

Ideally I would also have changed Timestamp:

  @Override
  public int compareTo(java.util.Date o) {
    if (o instanceof Timestamp) {
      return compareTo((Timestamp) o);
    } else {
      return compareTo(new Timestamp(o.getTime()));
    }
  }

to:

      return compareTo(new Timestamp(o.jsdate.getTime()));

but there is some kind of API validator claiming that jsdate is not visible in this case (even though it is for our emulated super class).

[manolo]

Try running your tests with the TZ=America/Los_Angeles env variable set