Change the panning and zooming behavior for React-Stockcharts tests

[bgrins]

As @squarewave mentioned in #11 (comment), it's unrealistic to send i.e. 50 wheel events to paint 50 frames inside of a single request animation frame callback (

Speedometer/resources/tests.mjs

Line 385 in f165570

new BenchmarkTestStep("PanTheChart", (page) => {

).

We could consider breaking them up into a handful of steps where we one or a few events. I'm not sure if this would then cause issues on fast desktop hardware where the steps become too short to be effectively measured, but we should take a close look at our options.

I don't know if this is directly contributing to what @camillobruni was seeing with bimodal results in Chrome, but I know there have been some concerns with that as well.

[bgrins]

In the meeting today there was consensus to make a change here. There was a concern expressed that splitting this up into one or a couple events per step might be too easy for fast hardware - but the next step is someone testing it out and making a recommendation.

[bgrins]

FYI - Julien is going to be taking a look at this in the next week or so

[julienw]

I gave some thoughts to this. Here is the result of this work: main...mozilla:Speedometer:split-react-stockcharts
This includes also my work from #132.

My feeling is that it's much better for the canvas+svg version, but the full svg version still suffers from missing work that happens in requestAnimationFrame callbacks, that sometimes is included in the async step, and sometimes isn't, and this creates bimodal results (at least in Firefox). I don't think this is solvable without an asynchronous runner.

Moreover, because of the synchronous runner, there's no paint happening between some steps when they should happen, most notably for between the wheel events. My understanding is that this impairs the behavior in Firefox (and possibly other browsers too? I haven't checked), because some work that usually happens during this async time (and therefore isn't a problem for a user) needs to happen "at some point later", interrupting the benchmark, and worsening the score, just because the benchmark isn't close enough to the real world.

I can create a pull request out of my work if there's interest in that. But I believe the stockcharts benchmarks in their current form should be removed because of their bimodality and the fact they simulate a non-realistic workload.

Happy to hear other thoughts!

[rniwa]

It appears to me we need to resolve this one or another before we can release the benchmark.

[bgrins]

It appears to me we need to resolve this one or another before we can release the benchmark.

I still think the best option is to just remove the tests, as we've been discussing over the months. These have had consistent issues with noise, and we are actively working on landing tests for much more popular charting and recently updated libraries (for SVG and Canvas, as we discovered in from our research). And if we do end up moving forward with something like #156 that adds even more canvas coverage.

I'd rather focus time and attention on getting others out of tentative than trying and figure these ones out. @rniwa @flashdesignory what do you think? Should we prepare a PR to remove them?

[rniwa]

When you say removing, are you talking about the whole React-Stockcharts suite or specific subtests?

I tend to agree that the current noise level in Chrome's results leaves this suite rather unsuitable to be included in the final benchmark in its current form. I'm fine with deleting it if we're adding more charting / React content elsewhere.

[bgrins]

When you say removing, are you talking about the whole React-Stockcharts suite or specific subtests?

I mean the whole suite

[rniwa]

When you say removing, are you talking about the whole React-Stockcharts suite or specific subtests?

I mean the whole suite

Okay, like I said above, I think I'm fine removing it. We've got a lot of other good contents already.

[julienw]

I'd still like to look whether we can make it more stable by mocking rAF. But I won't be able to look before Tuesday.

[bgrins]

I'd still like to look whether we can make it more stable by mocking rAF. But I won't be able to look before Tuesday.

Even if we can make them more stable with the mocked rAF I'm not sure it makes sense to bring them in based on our coverage with more popular charting libraries and other canvas/svg content. I'd like to hear from Thorsten as well, but if you want to take a look at stability next week we can wait to make a final decision until then.

[flashdesignory]

In general I'm ok with dropping react stockcharts, but I also want to give Julien enough time to do his thing.
Let's decide during our sync on Wednesday?