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

Replace NumberFormat with Ryu algorithm #30

Merged
merged 1 commit into from Jan 10, 2021
Merged

Replace NumberFormat with Ryu algorithm #30

merged 1 commit into from Jan 10, 2021

Conversation

ghost
Copy link

@ghost ghost commented Sep 7, 2020

Changes:

  • SVGGraphics2D.java -- Replace NumberFormat for both geometry and transform to use Ryu algorithm. Move constructor member variable initialization to declaration member variable initialization to avoid minor variable reassignment duplication. Collapses imports (the IDE did this automatically, sorry; although, modern IDEs handle import management very well these days). Set default value of geometryDP (2) and transformDP (6). Sprinkle final modifiers for method parameters.
  • RoundingMode.java -- Probably not needed because the mode is always ROUND_EVEN. Could be a clean up item to remove the enum and update the Ryu algorithm accordingly.
  • RyuDouble.java -- New double to string conversion implementation. Removed debug statements from the original code.
  • RyuFloat.java -- Not used, but I did not check to see whether there were other places that use the NumberFormat class, so this class can probably be deleted.

Comparative benchmarks when generating 1,000,000 SVGs from TeX expressions, 1 run each, no warm-up:

  • Baseline: 778060 milliseconds (~13 minutes)
  • Ryu: 421160 milliseconds (~7 minutes)

Here's a before and after profiling snapshot of the application's hot spot:

--- 3089912936 ns (10.04%), 309 samples
  [ 0] jdk.internal.math.FloatingDecimal$BinaryToASCIIBuffer.dtoa
  [ 1] jdk.internal.math.FloatingDecimal.getBinaryToASCIIConverter
  [ 2] jdk.internal.math.FloatingDecimal.getBinaryToASCIIConverter
  [ 3] java.text.DigitList.set
  [ 4] java.text.DecimalFormat.doubleSubformat
  [ 5] java.text.DecimalFormat.format
  [ 6] java.text.DecimalFormat.format
  [ 7] java.text.NumberFormat.format
  [ 8] org.jfree.svg.SVGGraphics2D.geomDP
  [ 9] org.jfree.svg.SVGGraphics2D.getSVGPathData
  [10] org.jfree.svg.SVGGraphics2D.fill
  [11] org.jfree.svg.SVGGraphics2D.drawGlyphVector
  [12] sun.font.ExtendedTextSourceLabel.handleDraw
  [13] sun.font.Decoration.drawTextAndDecorations
  [14] sun.font.ExtendedTextSourceLabel.draw
  [15] java.awt.font.TextLine.draw
  [16] java.awt.font.TextLayout.draw
  [17] org.jfree.svg.SVGGraphics2D.drawString
  [18] org.jfree.svg.SVGGraphics2D.drawString
  [19] org.jfree.svg.SVGGraphics2D.drawString
  [20] java.awt.Graphics.drawChars

And after:

--- 7429730977 ns (23.92%), 743 samples
  [ 0] org.jfree.svg.util.RyuDouble.doubleToString
  [ 1] org.jfree.svg.SVGGraphics2D.geomDP
  [ 2] org.jfree.svg.SVGGraphics2D.getSVGPathData
  [ 3] org.jfree.svg.SVGGraphics2D.fill
  [ 4] org.jfree.svg.SVGGraphics2D.drawGlyphVector
  [ 5] sun.font.ExtendedTextSourceLabel.handleDraw
  [ 6] sun.font.Decoration.drawTextAndDecorations
  [ 7] sun.font.ExtendedTextSourceLabel.draw
  [ 8] java.awt.font.TextLine.draw
  [ 9] java.awt.font.TextLayout.draw
  [10] org.jfree.svg.SVGGraphics2D.drawString
  [11] org.jfree.svg.SVGGraphics2D.drawString
  [12] org.jfree.svg.SVGGraphics2D.drawString
  [13] java.awt.Graphics.drawChars

Before:

          ns  percent  samples  top
  ----------  -------  -------  ---
  5989824368   19.46%      599  jdk.internal.math.FloatingDecimal$BinaryToASCIIBuffer.dtoa
  3949933992   12.83%      395  java.text.DecimalFormat.subformatNumber
  1799922822    5.85%      180  java.text.DigitList.round

And after:

          ns  percent  samples  top
  ----------  -------  -------  ---
 13429506398   43.24%     1343  org.jfree.svg.util.RyuDouble.doubleToString
  2109912262    6.79%      211  org.jfree.svg.SVGGraphics2D.geomDP

Further optimizations may be possible, such as removing the scientific notation logic from the Ryu algorithm provided it isn't used by the SVG standard.

@ghost ghost mentioned this pull request Sep 7, 2020
@mhschmieder
Copy link

mhschmieder commented Sep 7, 2020

Interesting results as well as problem approach. I don't think I had seen Ryu yet, though I remember seeing a less complete library along those lines a few years ago and a few years ago I "grew my own" solution to some of the problems addressed by a few of those methods (I was on the verge of publishing these as part of a small math library last month).

I hope to find time to review this in more depth later, as my specialty in graduate school was Numerical Analysis and so I have a real passion for evaluating and comparing the efficiency and precision of libraries that deal with numeric conversions of all types.

As the Ryu code was pulled in directly, probably it doesn't break any rules, but I'm too tired to look those up right now. My recollection is that the goal of each of these JFree projects is to not have outside dependencies, but that seems to be met (and I could be wrong).

The style changes are a mixed bag in terms of my own personal preferences as well as what may be established coding conventions within the JFree family (the overall code base is becoming much more consistent lately).

Regardless of what is decided by the project organizers, I appreciate the time and commitment that you put into this, and there are several categories of lessons that are valuable and may help in other contexts as well.

@ghost
Copy link
Author

ghost commented Sep 8, 2020

Updates correct geometry precision error and improper truncation of return value.

@jfree jfree merged commit b3aef96 into jfree:master Jan 10, 2021
@jfree
Copy link
Owner

jfree commented Jan 10, 2021

Nice work, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants