performance: implement caching for roundDecimal() and prevent unnecessary recalculations

[Connum]

Description

Implement caching for decimal rounding by storing the results for the decimal part of floats only and adding them to the significant part. Also avoid unnecessary recalculations by storing the value for reuse.

fixes #641

Motivation and Context

Using our performance test tool, we could see a huge drop (~1s vs ~6s) in performance from 1.3.4 to the current master. The culprit seems to have been the function roundDecimal() in path.js alone. That calculation seems to be very performance-hungry, however, it's the only way that I found so far to get reliable rounding of floats without losing precision.

How Has This Been Tested?

Using the performance test tool in docs/benchmark.html comparing to version 1.3.4 before and after the optimization. Also ran npm run test as well as the unicode test tool to make sure it doesn't break passing tests.

Screenshots (if appropriate):

before

after

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• I did npm run test and all tests passed green (including code styling checks).
• I have added tests to cover my changes. (not applicable)
• My change requires a change to the documentation.
• I have updated the README accordingly.
• I have read the CONTRIBUTING document.

[yne]

I'm still puzzled how a hashtable lookup can be faster that truncating a stringified float.

Is this truncating function really needed for standard compliance ? or is it for a cosmetic purpose ?

floatToString alone seems to call 3 times this function with the same argument each times, so another improvement would have be to use a variable once.

anyway, it's good to see some love about performance.

[Connum]

The costly part is +(Math.round(decimalPart + 'e+' + places) + 'e-' + places). So it's not a truncation or a simple Math.round(), because that will result in loss of precision. The rounding is done for every value of every point/command in every glyph in the string that is to be rendered. So I'm not really surprised this is quite costly! 😃

[Connum]

floatToString alone seems to call 3 times this function with the same argument each times, so another improvement would have be to use a variable once.

I'm already storing that in the variable rounded

[ILOVEPIE]

I'll take a look at this after my system's done updating.

[yne]

The costly part is +(Math.round(decimalPart + 'e+' + places) + 'e-' + places). So it's not a truncation or a simple Math.round(), because that will result in loss of precision. The rounding is done for every value of every point/command in every glyph in the string that is to be rendered. So I'm not really surprised this is quite costly! 😃

I'm struggling to understand the need for a loss precision function. can you show me an example of expected input-output for this ? hopefully it will help me grasp the idea.

[Connum]

The costly part is +(Math.round(decimalPart + 'e+' + places) + 'e-' + places). So it's not a truncation or a simple Math.round(), because that will result in loss of precision. The rounding is done for every value of every point/command in every glyph in the string that is to be rendered. So I'm not really surprised this is quite costly! 😃

I'm struggling to understand the need for a loss precision function. can you show me an example of expected input-output for this ? hopefully it will help me grasp the idea.

I tried out several rounding methods at the time, starting with a simple Math.round() in conjunction with multiplicater and subsequent division in order to round to a specific number of decimals. That resulted in wrong values e.g. when running the unicode test suite, as far as I remember. So did toFixed().

Here is a short blog post about the issues with floating-point arithmetics: https://www.jacklmoore.com/notes/rounding-in-javascript/
It's from 2014 and suggests the same approach we're using, but I haven't found anything more reliable.

[ILOVEPIE]

It might be possible to do this using a typed array/array buffer to do it correctly.

[yne]

I did not expect the hash give such performance boost,
but here is some bench I ran:

base	AVG
1.3.4	1353.3ms
master	5845.8ms
master + use .toFixed	3198ms
master + single call roundDecimal	3661.6ms
master + single call roundDecimal + hash	1695.3ms

In fact toFixed is slower than roundDecimal: