Chris Jones [:cjones] inactive; ni?/f?/r? if you need me Assignee
	Description • 12 years ago

The ioctls and associated overhead from galloc-buffer locking is taking ~5% of a profile of dragging the homescreen, even though we almost never repaint anything.  This is unnecessary and easy to fix.

Matt has a WIP patch.

	Andrew Overholt [:overholt]
	Comment 1 • 12 years ago

Chris, I nommed the dependencies of the 60fps bug but perhaps shouldn't have done so since making decisions about them is difficult :)  Any thoughts as to whether or not they should individually block?

	Chris Jones [:cjones] inactive; ni?/f?/r? if you need me Assignee
	Comment 2 • 12 years ago

As I mentioned in the other bug, setting blocking+ for the dependencies doesn't really make sense because we wouldn't hold the release for any of them individually.

The algorithm is going to be fix the known jank in order of best benefit/cost ration until we hit 60fps.  I lean towards them not individually blocking because I think it gives the wrong impression.

	Andrew Overholt [:overholt]
	Comment 3 • 12 years ago

(In reply to Chris Jones [:cjones] [:warhammer] from comment #2)
> I lean towards them not
> individually blocking because I think it gives the wrong impression.

Okay, thanks.

	Chris Jones [:cjones] inactive; ni?/f?/r? if you need me Assignee
	Comment 4 • 12 years ago

I started working on this and realized it's actually much harder than it seems on first blush, because ThebesLayerBuffer::BeginPaint() is how we determine whether or not there's work to do in a given PAINT phase, but that function requires the buffer to be mapped.

To really fix this, we'd need some sort of buffer "promise" (gfxASurface) that could map the buffer when it was actually about to be used, instead of checked against nullptr.  But also was nullptr when there's no buffer.

This patch adds the gymnastics to request buffer mapping from within ThebesLayerBuffer, but I didn't try to compile it.

	Chris Jones [:cjones] inactive; ni?/f?/r? if you need me Assignee
	Comment 5 • 12 years ago

I suspect what's happening here is that these ioctl()s are us taking and releasing genlock locks.  I further suspect that the actual work being done is quite cheap, but going into the kernel creates a preemption point and we get switched out for other runnable tasks.  But we shouldn't give up our time slice for unnecessary work, so should still fix this.

	Chris Jones [:cjones] inactive; ni?/f?/r? if you need me Assignee
	Comment 6 • 12 years ago

Comment on attachment 658408 [details] [diff] [review]
WIP

This is not the way I want to go.

	Chris Jones [:cjones] inactive; ni?/f?/r? if you need me Assignee
	Comment 7 • 12 years ago

OK!  This actually ended up being as easy as I hoped it would be.

This gets all the GraphicBuffer::lock/unlock ioctls out of the profile

http://people.mozilla.com/~bgirard/cleopatra/?report=33ac30f2e8076314694e41c22bda478e6c333d90

and wins us 4-5 fps on the homescreen.

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 8 • 12 years ago

Comment on attachment 660252 [details] [diff] [review]
Avoid mapping/unmapping buffers when we don't use them

Review of attachment 660252 [details] [diff] [review]:
-----------------------------------------------------------------

cool

	Chris Jones [:cjones] inactive; ni?/f?/r? if you need me Assignee
	Comment 9 • 12 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/b3bf5f7f02b9

	Ryan VanderMeulen [:RyanVM]
	Comment 10 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/b3bf5f7f02b9