1858310 - Calling Math functions is slow in development builds of the Firefox profiler

Status: NEW

(Core :: JavaScript Engine: JIT, defect, P2)

Description

[Markus Stange [:mstange]]

While optimizing the performance of the Firefox Profiler UI, I came across some surprising performance cliffs involving methods on Math, e.g. Math.floor and Math.abs.

The bad performance happens in a local development build of the profiler. I have not seen it happen in production builds. It is entirely possible that something in the profiler UI's development build pipeline adds some "go slow" behavior, but I'm not sure how to find out what that might be.

Steps to reproduce:

1. Clone this PR locally of the profiler repo, i.e. the slow-math branch of my fork at https://github.com/mstange/perf.html .
2. On the command line in the repo root directory, run yarn install && yarn start
3. Go to this URL: http://localhost:4242/public/rj5fsez7z9z4ewj4frmeabm02r7dm754ah2mh9g/calltree/?globalTrackOrder=0&thread=0&transforms=fs-m--async%2C-sync&v=10
4. In the call tree, press and hold the up button on your keyboard, and then press and hold the down button on your keyboard

Expected result:
The call tree selection should update relatively quickly.

Actual results:
Profile: https://share.firefox.dev/3rRBHv1
The first 24 selection changes are relatively fast, and then it starts to become really slow. The slowness is encountered when drawing the activity graph at the top, which makes use of Math.max, Math.min and Math.floor in a JS function called _accumulateInBuffer. After 24 selection changes, _accumulateInBuffer starts to spend a lot of time in js::jit::IonGetNameIC::update.

---

Alternative steps to reproduce:

1. Drag a selection in the timeline at the top, and keep dragging back and forth.

Eventually the dragging becomes slow. Profile: https://share.firefox.dev/3ZT53pK
Again the time is spent in js::jit::IonGetNameIC::update, this time in a JS function called computeCallTreeCountsAndSummary which makes use of Math.abs.

---

Alternative steps to reproduce:

1. Switch to the flame graph.
2. Move your mouse around in circles, to make different boxes redraw with hover colors.

Profile: https://share.firefox.dev/3PR6cJN
Same problem as above, this time with a Math.floor call in a JS function called snap.

Comment 1

[Jan de Mooij [:jandem]]

That's unexpected. I'll take a look, maybe something in the pipeline injects some weird JS code that confuses our name analysis.

Comment 2

[Jan de Mooij [:jandem]]

We keep bumping the global's generation count here because a property is being changed from accessor to data property and back. This keeps invalidating ICs for the Math lookup (and others).

This is for the window.event property that gets set in invokeGuardedCallbackDev. In restoreAfterDispatch, this ends up doing a DefineProperty that changes it to a data property:

  window.event = windowEvent;

But in invokeGuardedCallbackDev this defines it as an accessor property:

  if (windowEventDescriptor) {
    Object.defineProperty(window, 'event', windowEventDescriptor);
  }

React source code: https://github.com/facebook/react/blob/537228f9fd703d18bea1f6d20fa0e5006b795c42/packages/shared/invokeGuardedCallbackImpl.js#L61-L71