Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Math.rint is very slow and needs to be translated to JS Math.round #9156

Closed
kirillp opened this issue Jul 16, 2015 · 5 comments
Closed

Math.rint is very slow and needs to be translated to JS Math.round #9156

kirillp opened this issue Jul 16, 2015 · 5 comments
Milestone

Comments

@kirillp
Copy link

kirillp commented Jul 16, 2015

Found in GWT Release 2.7, trunk
Encountered on OS / Browser (e.g. WinXP, IE9, FF10): All

Detailed description (please be as specific as possible):

Math.rint is very slow.
This happens because it goes via slow code path and unnecessary conversion via long (call to "long round(double)", long is not supported natively by JS and is emulated in an complex and heavy way )

Shortest code snippet which demonstrates issue (please indicate where
actual result differs from expected result): call to Math.rint

patch:

+  public static native double rint(double x) /*-{
+   return Math.round(x);
+  }-*/;

-  public static double rint(double d) {
-    if (Double.isNaN(d)) {
-      return d;
-    } else if (Double.isInfinite(d)) {
-      return d;
-    } else if (d == 0.0d) {
-      return d;
-    } else {
-      return round(d);
-    }
-  }
@kirillp
Copy link
Author

kirillp commented Jul 16, 2015

@kirillp
Copy link
Author

kirillp commented Jul 16, 2015

The latest JDK documentation says:

Returns the double value that is closest in value
to the argument and is equal to a mathematical integer. If two
double values that are mathematical integers are
equally close, the result is the integer value that is
even.

So that is not exactly what JS Math.round does.

@tbroyer
Copy link
Member

tbroyer commented Jul 16, 2015

So that is what JS Math.round does.

No.

> Math.round(4.5)
5
> Math.round(3.5)
4

Math.rint(4.5) in Java would return 4.0, not 5.0 (“If two double values that are mathematical integers are equally close, the result is the integer value that is even”)

But the current implementation/emulation is already inaccurate as it delegates to round(), and making it accurate would probably be too costly at runtime (probably the reason it's done that way currently).

@kirillp
Copy link
Author

kirillp commented Jul 16, 2015

Suggested change will significantly speed-up current code.

@tbroyer tbroyer added this to the 2.8 milestone Apr 15, 2016
@tbroyer
Copy link
Member

tbroyer commented Apr 15, 2016

@tbroyer tbroyer closed this as completed Apr 15, 2016
niloc132 pushed a commit to niloc132/gwt-playground that referenced this issue May 9, 2016
Also fixes the NaN handling in Assert.java that I discovered
during testing.

Authors: Goktug Gokdogan, Andrei Korzhevskii
Bug: #9156
Bug-Link: gwtproject/gwt#9156
Change-Id: I2effe1c5ace329b2f708c53a1f83a295da4f5709
jDramaix pushed a commit to google/j2cl that referenced this issue Jun 20, 2017
commit ac5d74820b313d17fc0bcab902195afba7f5c59c
Author: Andrei Korzhevskii <a.korzhevskiy@gmail.com>
Date:   Thu Mar 24 12:01:35 2016 +0200

    Add java.lang Java 8 API

    This patch does not include new functional methods
    and unsigned methods in Number implementations.
    Some code and tests adapted from google guava libraries.

    Change-Id: I5f2ca3069de73ad8d8801d8e7bca7ebb1ed36e43

commit 0060ad8b3ce4db906c2adb179da066f086edea9a
Author: Andrei Korzhevskii <a.korzhevskiy@gmail.com>
Date:   Thu Mar 24 9:33 2016 +0200

    Fixes Math.rint.

    Also fixes the NaN handling in Assert.java that I discovered
    during testing.

    Authors: Goktug Gokdogan, Andrei Korzhevskii
    Bug: #9156
    Bug-Link: gwtproject/gwt#9156
    Change-Id: I2effe1c5ace329b2f708c53a1f83a295da4f5709

commit fa1a1bf057bd4d28dfb8c1590a9a59df396604d3
Author: Andrei Korzhevskii <a.korzhevskiy@gmail.com>
Date:   Thu Apr 20 11:08 2016 +0200

    Handle NaN and -0.0 in Math.signum

    Change-Id: I0aae67a00fb99ac61c17aa80e595cd689e5d18d8

commit b8dbec48afab7b5de3beb0fd52911370acd2bfad
Author: Goktug Gokdogan <goktug@google.com>
Date:   May 13 2016

    Fix handling of -0 in Math.copySign

    Change-Id: I3cbae4d357763a96400c0e3ada7646b151728ba9

commit 64da754d649bdfbeb7e2707f57c45706b26811a3
Author: Andrei Korzhevskii <a.korzhevskiy@gmail.com>
Date:   May 15 2016 +0200

    Fix Math.expm1 implementation

    Bug: #9331
    Change-Id: I65639c4462155a07333d35d91f2e69f66c62489a
    Bug-Link: gwtproject/gwt#9331

commit dfcc65d1b96a4c37d3157afb35ce6d820a08bedb
Author: Andrei Korzhevskii <a.korzhevskiy@gmail.com>
Date:   May 20 2016 +0200

    Use native code for Math.abs

    Change-Id: I3f917505736da67d0b182b9e83612cb09712bc49

commit 93c726a2b7e9c3a8a970f9f669bf357496e9dfc6
Author: Andrei Korzhevskii <a.korzhevskiy@gmail.com>
Date:   May 25 2016 +0200

    Refactor newly added Math.floorDiv, floorMod, multiplyExact methods.

    Change-Id: Ie5cffa85ec296b7f44776149a0d2c0ab4a77fb33

commit c32d7d1958e4b176a4bd3e1500a08dd7942ac784
Author: Andrei Korzhevskii <a.korzhevskiy@gmail.com>
Date:   May 27 2016 +0200

    Fix tests for Math.round and Math.floor

    Change-Id: Iecb0f9888c3ca382f176cbc14e88634f8a50e509

commit 104750cda822763852b41f629f5fa084bb81b16c
Author: Andrei Korzhevskii <a.korzhevskiy@gmail.com>
Date:   Jun 1 2016 +0200

    Fix Math.rint for IE and htmlunit for numbers >= 2^52

    Change-Id: Ib8631b19b2018dbe216782aee92d4c081250401d

commit 1587f22cd746d7d87e76bcb3c79c91aecffb46d7
Author: Daniel Kurka <dankurka@google.com>
Date:   Jun 1 2016 +0200

    Move tests that require java8 into a java8 suite.

    Change-Id: Ic69b6c88a80f8e54b7ff979d18c381e2b2e254d6

PiperOrigin-RevId: 123774891
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants