From 273ba62e0ef503b8037e0e89543e288f810ad85a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mime=20=C4=8Cuvalo?= Date: Mon, 15 Apr 2024 19:45:30 +0100 Subject: [PATCH] perf: calculate hypoteneuse manually instead of using hypot (#3468) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Something was bothering me a bit with the discussion around sqrt's being slow. Looks like `Math.hypot` has a performance cost associated with it. Looking at the Chromium source code: https://chromium.googlesource.com/v8/v8/+/4.3.21/src/math.js?autodive=0%2F%2F#19 and https://source.chromium.org/chromium/chromium/src/+/main:v8/src/builtins/math.tq;l=36?q=math&sq=&ss=chromium%2Fchromium%2Fsrc:v8%2Fsrc%2F it looks like maybe we'd be avoiding the multiple arguments that can be passed into Math.hypot which is maybe the source of the perf hit. Also, interestingly in `math.tq` you can see it doing this funky sqrt calculation: `Float64Sqrt((a / max) * (a / max) + (b / max) * (b / max)) * max` - I think that possibly is trying to avoid some overflow in some cases with bigger numbers, but also possibly with a perf hit. [edit]: OK, actually on Firefox, doing sqrt seems slower - but digging more into this, it looks like doing `** 0.5` instead of `sqrt` is much faster. More related articles: - https://stackoverflow.com/questions/71898044/why-is-math-hypot-so-slow - https://stackoverflow.com/questions/3764978/why-hypot-function-is-so-slow - https://www.reddit.com/r/javascript/comments/wk3e57/askjs_why_mathsqrt_is_so_slow_in_firefox/ [edit again!] looks like this is being fixed in the latest Chrome! https://blog.seokho.dev/development/2024/03/18/V8-optimize-MathHypot.html ``` ┌─────────┬───────┬─────────┬─────────┬─────────┬────────┐ │ (index) │ Cold │ Slowest │ Fastest │ Average │ Total │ ├─────────┼───────┼─────────┼─────────┼─────────┼────────┤ │ old │ 13.39 │ 10.07 │ 9.69 │ 9.98 │ 998.57 │ │ sqrt │ 8.19 │ 6.66 │ 6.61 │ 6.67 │ 667.6 │ │ pow 0.5 │ 1.89 │ 0.28 │ 0.28 │ 0.3 │ 29.79 │ │ new │ 1.64 │ 0.28 │ 0.28 │ 0.29 │ 28.95 │ └─────────┴───────┴─────────┴─────────┴─────────┴────────┘ ``` ### Change Type - [x] `sdk` — Changes the tldraw SDK - [ ] `dotcom` — Changes the tldraw.com web app - [ ] `docs` — Changes to the documentation, examples, or templates. - [ ] `vs code` — Changes to the vscode plugin - [ ] `internal` — Does not affect user-facing stuff - [ ] `bugfix` — Bug fix - [ ] `feature` — New feature - [x] `improvement` — Improving existing features - [ ] `chore` — Updating dependencies, other boring stuff - [ ] `galaxy brain` — Architectural changes - [ ] `tests` — Changes to any test code - [ ] `tools` — Changes to infrastructure, CI, internal scripts, debugging tools, etc. - [ ] `dunno` — I don't know --- .../src/lib/editor/shapes/shared/arrow/curved-arrow.ts | 6 +++--- packages/editor/src/lib/primitives/Mat.ts | 8 ++++---- packages/editor/src/lib/primitives/Vec.ts | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/editor/src/lib/editor/shapes/shared/arrow/curved-arrow.ts b/packages/editor/src/lib/editor/shapes/shared/arrow/curved-arrow.ts index 03e3c2ad4..a8a755df5 100644 --- a/packages/editor/src/lib/editor/shapes/shared/arrow/curved-arrow.ts +++ b/packages/editor/src/lib/editor/shapes/shared/arrow/curved-arrow.ts @@ -389,9 +389,9 @@ function getArcInfo(a: VecLike, b: VecLike, c: VecLike): TLArcInfo { const sweepFlag = +Vec.Clockwise(a, c, b) // The base angle of the arc in radians - const ab = Math.hypot(a.y - b.y, a.x - b.x) - const bc = Math.hypot(b.y - c.y, b.x - c.x) - const ca = Math.hypot(c.y - a.y, c.x - a.x) + const ab = ((a.y - b.y) ** 2 + (a.x - b.x) ** 2) ** 0.5 + const bc = ((b.y - c.y) ** 2 + (b.x - c.x) ** 2) ** 0.5 + const ca = ((c.y - a.y) ** 2 + (c.x - a.x) ** 2) ** 0.5 const theta = Math.acos((bc * bc + ca * ca - ab * ab) / (2 * bc * ca)) * 2 diff --git a/packages/editor/src/lib/primitives/Mat.ts b/packages/editor/src/lib/primitives/Mat.ts index c895ded76..b2388fcb1 100644 --- a/packages/editor/src/lib/primitives/Mat.ts +++ b/packages/editor/src/lib/primitives/Mat.ts @@ -218,10 +218,10 @@ export class Mat { let rotation if (m.a !== 0 || m.c !== 0) { - const hypotAc = Math.hypot(m.a, m.c) + const hypotAc = (m.a * m.a + m.c * m.c) ** 0.5 rotation = Math.acos(m.a / hypotAc) * (m.c > 0 ? -1 : 1) } else if (m.b !== 0 || m.d !== 0) { - const hypotBd = Math.hypot(m.b, m.d) + const hypotBd = (m.b * m.b + m.d * m.d) ** 0.5 rotation = HALF_PI + Math.acos(m.b / hypotBd) * (m.d > 0 ? -1 : 1) } else { rotation = 0 @@ -234,12 +234,12 @@ export class Mat { let scaleX, scaleY, rotation if (m.a !== 0 || m.c !== 0) { - const hypotAc = Math.hypot(m.a, m.c) + const hypotAc = (m.a * m.a + m.c * m.c) ** 0.5 scaleX = hypotAc scaleY = (m.a * m.d - m.b * m.c) / hypotAc rotation = Math.acos(m.a / hypotAc) * (m.c > 0 ? -1 : 1) } else if (m.b !== 0 || m.d !== 0) { - const hypotBd = Math.hypot(m.b, m.d) + const hypotBd = (m.b * m.b + m.d * m.d) ** 0.5 scaleX = (m.a * m.d - m.b * m.c) / hypotBd scaleY = hypotBd rotation = HALF_PI + Math.acos(m.b / hypotBd) * (m.d > 0 ? -1 : 1) diff --git a/packages/editor/src/lib/primitives/Vec.ts b/packages/editor/src/lib/primitives/Vec.ts index 48beec6c1..898e0578f 100644 --- a/packages/editor/src/lib/primitives/Vec.ts +++ b/packages/editor/src/lib/primitives/Vec.ts @@ -315,7 +315,7 @@ export class Vec { // Get the distance between two points. static Dist(A: VecLike, B: VecLike): number { - return Math.hypot(A.y - B.y, A.x - B.x) + return ((A.y - B.y) ** 2 + (A.x - B.x) ** 2) ** 0.5 } // Get whether a distance between two points is less than a number. This is faster to calulate than using `Vec.Dist(a, b) < n`. @@ -355,7 +355,7 @@ export class Vec { } static Len(A: VecLike): number { - return Math.hypot(A.x, A.y) + return (A.x * A.x + A.y * A.y) ** 0.5 } /**