23.5% linux 32|64 GLTerrain regression on inbound (v.35) Oct 10 from push 216915390f9b

RESOLVED WONTFIX

Status

()

Core
Canvas: WebGL
RESOLVED WONTFIX
3 years ago
3 years ago

People

(Reporter: jmaher, Assigned: jgilbert)

Tracking

({perf, regression})

unspecified
perf, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox36+ wontfix, firefox37- affected, firefox38 affected, firefox39 affected)

Details

(Whiteboard: [talos_regression][gfx-noted])

(Reporter)

Comment 1

3 years ago
Jeff, can you look into this. I would like to know:
* is this expected?
* why is this so large on linux and winxp
* this will be on Aurora, any fixes need to go there, will that be a problem?
* does the test need to be modified?
Flags: needinfo?(jgilbert)
(Assignee)

Comment 2

3 years ago
(In reply to Joel Maher (:jmaher) from comment #1)
> Jeff, can you look into this. I would like to know:
> * is this expected?
No.
> * why is this so large on linux and winxp
Because they do readback, and I believe this patch makes them readback twice. The fix for this is simple. (bug 1081363)
> * this will be on Aurora, any fixes need to go there, will that be a problem?
No, the fix is pretty simple.
> * does the test need to be modified?
No.

The Win7+ perf hit is something I'm looking into.
Flags: needinfo?(jgilbert)
(Assignee)

Updated

3 years ago
See Also: → bug 1081363
(Reporter)

Comment 3

3 years ago
thanks Jeff!
(Assignee)

Comment 4

3 years ago
With bug 1081363, we're actually improved our WinXP results (~22ms) since before the regression (~27ms).
Linux results are down from the regression peak, but still above pre-regression levels. Linux perf is still known-bad since we don't have accelerated layers there, so this is probably acceptable for the short term.

Win7+ results aren't affected by bug 1081363. I suspect the Win7 regression might be from how we new create new TexClients every frame. This would suggest that the Win7 results should be viewed in terms of absolute ms difference (~0.3ms/frame) rather than as a percentage regression. I'll be working on a fix for this this week, so we should be able to check this 'soon'.

If we had a test parallel to this which was just more taxing, it would be simpler to tell if my theory above is correct. Even just running this existing test again but drawing twice per frame would highlight this behavior.
(Assignee)

Updated

3 years ago
Assignee: nobody → jgilbert

Comment 5

3 years ago
(In reply to Jeff Gilbert [:jgilbert] from comment #4)
> ...
> If we had a test parallel to this which was just more taxing, it would be
> simpler to tell if my theory above is correct. Even just running this
> existing test again but drawing twice per frame would highlight this
> behavior.

If you feel that the test should be more taxing in general, or that we should have another, more taxing, variant of it running in parallel, you're welcome to modify it or to create a new variant of it.

Otherwise, if it's just to confirm this theory but not with much value in general, you could always run some benchmark locally with builds before/after the patch and confirm your theory.

If then you think the theory is confirmed and the apparent regression is acceptable to you, then that's a valid conclusion.

These tests are for the gfx team more than for anyone else. As long as you you understand the results, we'll be fine with whatever conclusions you arrive at...
(Reporter)

Comment 6

3 years ago
Right now win7 and win8 show no signs of fixing, but I do see the winxp fix and the partial fix for linux32 and linux64.

Do we feel that bug 1081363 is done enough that we can uplift to Aurora?

Regarding linux and the short term, are we talking a couple of weeks?

I want to make sure we are aware that this is on Aurora and we have ~10 weeks until it is shipped.
(Assignee)

Comment 7

3 years ago
(In reply to Joel Maher (:jmaher) from comment #6)
> Right now win7 and win8 show no signs of fixing, but I do see the winxp fix
> and the partial fix for linux32 and linux64.
> 
> Do we feel that bug 1081363 is done enough that we can uplift to Aurora?
> 
> Regarding linux and the short term, are we talking a couple of weeks?
> 
> I want to make sure we are aware that this is on Aurora and we have ~10
> weeks until it is shipped.

No, I backed 1066280 out of Aurora earlier today. It's not worth taking in the uplift in this state.
I'll continue to track it for Trunk, though.
(Reporter)

Comment 8

3 years ago
great, do let me know if you have questions or need any help with any testing or analyzing results.
(Reporter)

Updated

3 years ago
Blocks: 1084461
(Reporter)

Comment 9

3 years ago
this is now on aurora with a ~23% hit for linux32 and linux64.  Winxp shows a 17% improvement!
(Reporter)

Updated

3 years ago
Summary: 49.5% linux|windows GLTerrain regression on inbound (v.35) Oct 10 from push 216915390f9b → 23.5% linux 32|64 GLTerrain regression on inbound (v.35) Oct 10 from push 216915390f9b
(Reporter)

Comment 10

3 years ago
this is now on mozilla-beta
Jeff, do you have plans for this bug for 36? Thanks
status-firefox36: --- → affected
tracking-firefox36: --- → +
Flags: needinfo?(jgilbert)
(Assignee)

Comment 12

3 years ago
(In reply to Sylvestre Ledru [:sylvestre] from comment #11)
> Jeff, do you have plans for this bug for 36? Thanks

No.
Flags: needinfo?(jgilbert)
OK, thanks.
Tracking for 37 in case we want to do something about this.
status-firefox36: affected → wontfix
status-firefox37: --- → affected
tracking-firefox37: --- → +
Hi Jeff, since you're the assignee and this is tracked for 37, is this something you can pick up in the next couple of weeks for 37 while it's in beta? Or is there someone else you could recommend to work on it?

Should we continue tracking this for 38 or is it something that's going to be dropped?  Thanks!
Flags: needinfo?(jgilbert)
(Assignee)

Comment 15

3 years ago
(In reply to Liz Henry :lizzard from comment #14)
> Hi Jeff, since you're the assignee and this is tracked for 37, is this
> something you can pick up in the next couple of weeks for 37 while it's in
> beta? Or is there someone else you could recommend to work on it?
> 
> Should we continue tracking this for 38 or is it something that's going to
> be dropped?  Thanks!

I'll check if it's something trivial, but I can't spend a week on it. Linux perf is low-priority for Graphics. I don't think it's worth tracking this given our priorities.
Flags: needinfo?(jgilbert)
I'm dropping tracking as this is a low priority item for the gfx team that can get prioritized with the rest of their work.
status-firefox38: --- → affected
status-firefox39: --- → affected
tracking-firefox37: + → -

Updated

3 years ago
Whiteboard: [talos_regression] → [talos_regression][gfx-noted]
(Reporter)

Comment 17

3 years ago
shall we close this as wontfix?  the code has already shipped.
(Reporter)

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.