1886982 - Number.toFixed() is slow

Status: NEW

(Core :: JavaScript: Standard Library, defect, P3)

Description

[Florian Quèze [:florian]]

While profiling the JavaScript code of a web page I'm making for an IoT project, I noticed that calls to .toFixed() were a significant part of the CPU time.

I artificially increased the size of the data that my page is processing to be able to easily see things in profile.

In this profile there are about 30 million calls made to .toFixed(). The total processing time for my makeSVGPath function is 2.7s.

When replacing .toFixed() with a call to Math.round, the total processing time drops to 0.9s: https://share.firefox.dev/3IMcEyi

If for an apple to apple comparison I use Math.round(number).toString(), the total time is 1.1s: https://share.firefox.dev/4coKSpp

Should there be a fast path for the case where toFixed is asked to produce 0 digits after the decimal point?

Comment 1

[André Bargull [:anba]]

When testing in shell µ-benchmarks, .toFixed() actually seems to be slightly faster for us when compared to JSC/V8, so it seems we don't have to worry about us being slower than the competition.

That being said, making .toFixed() faster will likely require:

1. For a minor improvement, the precision = 0 case can probably be optimised to NumberToString<CanGC>(cx, std::copysign(math_round_impl(std::abs(d)), d)), as long as the input d isn't too large. (The input mustn't be too large to correctly handle 100000000000000100..toFixed(0) == "100000000000000096", whereas 100000000000000100..toString() == "100000000000000100".)
2. There's still a noticeable VM-call overhead, so toFixed() needs to be inlined in CacheIR/Warp to close the gap when compared to Math.round(number).toString().

The profile also shows that Math.min(...ys) and Math.max(...ys) aren't inlined, probably because ys is too large. (I think this is an optimisation bug when we decide to inline the native function. Filed as bug 1888418.)