Status

RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: cjones, Assigned: wesj)

Tracking

Trunk
x86_64
Linux
Dependency tree / graph

Details

(Whiteboard: [part 1 landed - more coming?])

Attachments

(2 attachments, 2 obsolete attachments)

There are two desiderata to be balanced

 (1) Larger displayport allows fast-panning longer in the chrome process before requesting re-rendering from content
 (2) Smaller displayport allows content to re-render more quickly

Tuning this size is really something we want to measure quantitatively over a benchmark suite.
Oops, there's also

 (3) Smaller displayport requires less memory
tracking-fennec: --- → ?
tracking-fennec: ? → 2.0+
Assignee: nobody → jones.chris.g
This is a fennec frontend project, out of my wheelhouse.
Assignee: jones.chris.g → nobody
Assignee: nobody → ben
I tried toolkit.browser.cacheRatioHeight", 4000, and it feels much better on simple page, at least single kinetic scroll in landscape mode does not show checkerboard.

time:		avail:	size:	rss:	clean:	shared:	private: change:
4000 height
content
20:14:06	203252	145096	47388	540	884	23988	   +0 kB
chrome
20:14:22	203280	156580	60100	11868	3080	20408	   +0 kB

2200 height
content:
20:11:53	197260	137784	44428	524	884	21040	   +0 kB
chrome:
20:11:38	197268	156828	61360	12012	3076	21620	   +0 kB
Duplicate of this bug: 630125
Assignee: ben → wjohnston
is this just a meta bug now?
Why do you think that?  Finding an optimal displayport for scrolling benchmark(s) while minimizing memory usage seems pretty well scoped.  What this project lacks is the scrolling benchmark(s).
(In reply to comment #5)
> is this just a meta bug now?

Nope. Getting new pref values, like Oleg tried in comment 3, is what this particular bug is about. Balancing memory use with decrease in checkerboarding.

We have metrics for memory, but objective metrics for panning / checkerboarding are lacking. We have been using subjective evaluation for checkerboarding.

We could add some timing code to see how much time is spent rendering the checkerboard background - I think it is only rendered as needed, so less checkerboarding means less time spent rendering it.

Until we have objective metrics, subjective will have to be good enough:
* How much vertical panning is checkerboard free?
* Zoom in and try vertical panning again
* How much horizontal panning is checkerboard free?
* Zoom in and try again
Duplicate of this bug: 638084
(Assignee)

Comment 9

8 years ago
Created attachment 516307 [details] [diff] [review]
Patch

Talked to stechz about this. He's done some pretty extensive panning in his days, and seems to think that just increasing the cache width will give us what we want.
Attachment #516307 - Flags: review?(mark.finkle)
Comment on attachment 516307 [details] [diff] [review]
Patch

I'm fine with this change - horizontal checkerboarding , especially while zoomed, is bad right now. I do want to now if we can extend the vertical direction as well.

Changing the height to 3000 or 4000 would help squash more checkerboarding, and it looks like memory usage does not go up very much.

Tell me why we don't need (or want) to do the vertical too
(In reply to comment #10)
> Changing the height to 3000 or 4000 would help squash more checkerboarding, and
> it looks like memory usage does not go up very much.
> 
> Tell me why we don't need (or want) to do the vertical too

As comment 0 notes, the other disadvantage of a too-large displayport is that it takes longer to redraw.  You'll run into checkerboard less often, but it may stick around longer when you do.  So we should test this before making any height increase.  (And we should re-evaluate after bug 634397 lands, since that will change the results.)
Depends on: 634397
(Assignee)

Comment 12

8 years ago
Created attachment 516426 [details] [diff] [review]
Patch

Oops. Wrong patch.
Attachment #516307 - Attachment is obsolete: true
Attachment #516426 - Flags: review?(21)
Attachment #516307 - Flags: review?(mark.finkle)
(Assignee)

Comment 13

8 years ago
Created attachment 516428 [details] [diff] [review]
Patch v1

Argh. nope. wrong bug.
Attachment #516426 - Attachment is obsolete: true
Attachment #516428 - Flags: review?(mark.finkle)
Attachment #516426 - Flags: review?(21)
Comment on attachment 516428 [details] [diff] [review]
Patch v1

Let's land this and get feedback on the change
Attachment #516428 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 15

8 years ago
http://hg.mozilla.org/mobile-browser/rev/0811c1cc3170

Wait to mark fixed? I am working on a different patch to help quantize checkerboard area shown/sec during a pan.
Status: NEW → ASSIGNED
Whiteboard: [needs-landing]
Actually height increase definitely needed + repainting visible area at first.
Whiteboard: [needs-landing] → [part 1 landed - more coming?]
what's the plan here? Can this be closed? Perhaps follow up work should be done in another bug.
(Assignee)

Comment 18

8 years ago
Given we are expecting big changes in checkerboarding, we will likely need to revisit this later.

Since we're just playing with prefs, I think the plan was to hold off on closing this until after that.
(Assignee)

Comment 19

8 years ago
Created attachment 518441 [details] [diff] [review]
Patch 2

This feels a better to me.
Attachment #518441 - Flags: review?(ben)
Comment on attachment 518441 [details] [diff] [review]
Patch 2

That's what I've been playing with too.
Attachment #518441 - Flags: review?(ben) → review+
(Assignee)

Comment 21

8 years ago
pushed: http://hg.mozilla.org/mobile-browser/rev/34ca5bea3ebe

closing this bug now.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Blocks: 640444
Is there a way to test this on a user-facing side?
Depends on: 644665
You need to log in before you can comment on or make changes to this bug.