1676694 - speed-battle.com shows a significant (JavaScript) perf drop between FF 82.0.3 and 83.0 RC1

Status: NEW

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

Description

[Arthur K. (he/him)]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:83.0) Gecko/20100101 Firefox/83.0

Steps to reproduce:

I'm running 82.0.3 on an old Dell 7020 and testing performance on speed-battle.com.

Actual results:

Scores for FF 82.0.3 range between 1510-1575 but drop to a range of around 525-575 using FF 83.0 RC1. Somethings definitely regressed here.

Expected results:

I would think scores would remain the range of 82.0.3 or improve slightly.

Comment 1

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4548a773cf089b4f42b83d990f3e6fa5f0c55743&tochange=91b890df65c564e5bbcd24b30ed576a045211fc5

Comment 2

[Jan de Mooij [:jandem]]

This looks like some kind of JS array micro-benchmark. The source code is in German.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1666417

Comment 4

[Tom S [:evilpie]]

This benchmark is quite unrepresentative. It seems like the author isn't familiar with JavaScript:

MeinArray = new Array(100,schleifenziel);
for (i=1; i<100; i++)
	{
	for (j=1; j<schleifenziel; j++)
		{
		MeinArray[i-1,j-1] = j; // ... No 2d arrays in JS ...
		}
	}

Around 30% of the time it's just doing sin/cos.

for (i=1; i<schleifenziel; i++)
	{
	a = Math.cos(i)*Math.sqrt(Math.sin(i))
	}

Comment 5

[Arthur K. (he/him)]

May be so but it's still regressed.

Comment 6

[Iain Ireland [:iain]]

In Firefox 83, we are replacing IonBuilder, the old front-end to our optimizing compiler, with WarpBuilder, a new design. You can find more information in this blog post we just published.

This code looks similar in style to the benchmarks we discuss in the "Synthetic JS Benchmarks" section of that blog post. We expect and accept regressions on this kind of code, in exchange for performance improvements on JS that is more representative of the average website. We intend to gradually improve Warp's performance on synthetic benchmarks, but it's not a high priority.

Looking at the code we generate for the two snippets Tom posted above (which are, AFAICT, the only two snippets that are being tested), Warp seems to be pretty similar to Ion in the sin/cos case. I suspect the difference is in the array case. Simplifying a bit, we have:

var arr = new Array();
for (var i = 0; i < 10000; i++) {
    arr[i] = i;
}

There are a couple of notable differences between Ion and Warp here.

First, when adding an indexed property, Warp has to shape guard the prototype chain. We might be able to do something fuse-related here eventually.

Second, Ion is smart enough to use StoreElementHoleT here, while Warp generates StoreElementHoleV and therefore needs a post-write barrier, even though we've already guarded that i is Int32 several times. We should be able to do better, either by updating slots after guarding them during WarpBuilder (similar to bug 1675084), or with a new optimization pass looking at dominating guards.

Comment 7

[Arthur K. (he/him)]

(In reply to Iain Ireland [:iain] from comment #6)

In Firefox 83, we are replacing IonBuilder, the old front-end to our optimizing compiler, with WarpBuilder, a new design. You can find more information in this blog post we just published.

This code looks similar in style to the benchmarks we discuss in the "Synthetic JS Benchmarks" section of that blog post. We expect and accept regressions on this kind of code, in exchange for performance improvements on JS that is more representative of the average website. We intend to gradually improve Warp's performance on synthetic benchmarks, but it's not a high priority.

Looking at the code we generate for the two snippets Tom posted above (which are, AFAICT, the only two snippets that are being tested), Warp seems to be pretty similar to Ion in the sin/cos case. I suspect the difference is in the array case. Simplifying a bit, we have:

var arr = new Array();
for (var i = 0; i < 10000; i++) {
    arr[i] = i;
}

There are a couple of notable differences between Ion and Warp here.

First, when adding an indexed property, Warp has to shape guard the prototype chain. We might be able to do something fuse-related here eventually.

Second, Ion is smart enough to use StoreElementHoleT here, while Warp generates StoreElementHoleV and therefore needs a post-write barrier, even though we've already guarded that i is Int32 several times. We should be able to do better, either by updating slots after guarding them during WarpBuilder (similar to bug 1675084), or with a new optimization pass looking at dominating guards.

I had seen on the blog post that some artificial benchmarks like on speed-battle.com would probably take a hit and yes, I agree, the new JIT should be middle of the road and, as you say, for the average website. While bug 1675084 probably addressed a couple things, speed-battle didn't improve while I've been testing the 84 betas.

Again, this is not an important benchmark but filing this bug was just an FYI regarding one spot where I observed a negative change and just wanted to draw attention to it. I imagine further improvements to Warp will arrive around down the road so I'll keep testing and reporting as time progresses.