Benoit Girard (:BenWa) Reporter
	Description • 13 years ago

Jeff doesn't expect the cairo option to cause a significant performance regression and is needed because of the shared buffers used by cairo. We need this option for off-main thread composting since we will be using cairo from the gecko and the compositing thread.

	Mozilla RelEng Bot
	Comment 3 • 13 years ago

Try run for e9d853679e7a is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=e9d853679e7a
Results (out of 142 total builds):
    exception: 13
    success: 100
    warnings: 21
    failure: 8
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-e9d853679e7a

	Mozilla RelEng Bot
	Comment 4 • 13 years ago

Try run for 60da465f2f5e is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=60da465f2f5e
Results (out of 27 total builds):
    success: 24
    warnings: 2
    failure: 1
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-60da465f2f5e

	Benoit Girard (:BenWa) Reporter
	Comment 5 • 13 years ago

This patch works everywhere except on win32 debug.

I'm torn between enabling this everywhere and getting this in ASAP for osx/android.

	Joe Drew (not getting mail)
	Comment 6 • 12 years ago

We're not using basic layers for Fennec.

	Mozilla RelEng Bot
	Comment 7 • 12 years ago

Try run for d585af23ebd3 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=d585af23ebd3
Results (out of 117 total builds):
    exception: 2
    success: 90
    warnings: 25
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-d585af23ebd3

	Marco Castelluccio [:marco]
	Comment 8 • 12 years ago

Comment on attachment 577691 [details] [diff] [review]
patch

https://tbpl.mozilla.org/?tree=Try&rev=2b8de579770a

	Benoit Girard (:BenWa) Reporter
	Comment 9 • 12 years ago

Excellent, I'll land this once inbound clears up.

	Benoit Girard (:BenWa) Reporter
	Comment 10 • 12 years ago

Comment on attachment 577691 [details] [diff] [review]
patch

The fixes marco landed make this patch work.

	Benoit Girard (:BenWa) Reporter
	Comment 11 • 12 years ago

Comment on attachment 577691 [details] [diff] [review]
patch

Taking of check-in, please re-request once the patch is r+.

	Oleg Romashin (:romaxa)
	Comment 13 • 12 years ago

Comment on attachment 577691 [details] [diff] [review]
patch

so can we push this fix?

	Jeff Muizelaar [:jrmuizel]
	Comment 14 • 12 years ago

It's not clear to me what we're doing this for yet.

	Oleg Romashin (:romaxa)
	Comment 15 • 12 years ago

see bug 832794, I catched reproducible case when I do create gfxContext in one thread, and destroy context in another thread, context refcount in first thread decreased by context destroy in another thread... (compositor and gecko threads OMTC case)

	Jeff Muizelaar [:jrmuizel]
	Comment 16 • 12 years ago

(In reply to Oleg Romashin (:romaxa) from comment #15)
> see bug 832794, I catched reproducible case when I do create gfxContext in
> one thread, and destroy context in another thread, context refcount in first
> thread decreased by context destroy in another thread... (compositor and
> gecko threads OMTC case)

It's certainly true that cairo is not thread safe. However, none of the configurations that we ship in require using cairo from multiple threads. I'm fine with making this a build option, but I don't think we should do it unconditionally until we need it.

	Benoit Girard (:BenWa) Reporter
	Comment 17 • 12 years ago

(In reply to Oleg Romashin (:romaxa) from comment #15)
> see bug 832794, I catched reproducible case when I do create gfxContext in
> one thread, and destroy context in another thread, context refcount in first
> thread decreased by context destroy in another thread... (compositor and
> gecko threads OMTC case)

What's your use case for having a gfxContext between threads? We don't have this in m-c AFAIK so it seems silly to take this perf hit for something we don't do.

	Oleg Romashin (:romaxa)
	Comment 18 • 12 years ago

I have it with embedding environment... I have possibility to run it with SW rendering, and I do create gfxContext from bitmap image source in CompositorParent thread.

	Benoit Girard (:BenWa) Reporter
	Comment 19 • 12 years ago

That sounds reasonable. Keep in mind that adding software based compositing is going to get much easier once the graphics branch merges with central carrying refactoring for how we deal with layers and compositing.

	Bill McCloskey [inactive unless it's an emergency] (:billm)
	Comment 20 • 11 years ago

Jeff, Would it be okay to do this now? This is making it hard to test e10s on mozilla-central, which is a big goal of ours.

	Jeff Muizelaar [:jrmuizel]
	Comment 21 • 11 years ago

(In reply to Bill McCloskey (:billm) from comment #20)
Yes. That seems fine.

	Bill McCloskey [inactive unless it's an emergency] (:billm)
	Comment 22 • 11 years ago

(In reply to Jeff Muizelaar [:jrmuizel] from comment #21)
> (In reply to Bill McCloskey (:billm) from comment #20)
> Yes. That seems fine.

Can you review then?

	Matt Woodrow (:mattwoodrow)
	Comment 23 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/5495c78f77ea

	Phil Ringnalda (:philor)
	Comment 24 • 11 years ago

Backed out in http://hg.mozilla.org/integration/mozilla-inbound/rev/6b789dbea375 to see if this is really what caused us to go from zero timeouts in ten rounds of linux32 crashtest-ipc on the push before to between one in three and two in three on this and subsequent pushes.

	Phil Ringnalda (:philor)
	Comment 25 • 11 years ago

Apparently it wasn't this, still timing out after backing it out.

	Phil Ringnalda (:philor)
	Comment 26 • 11 years ago

Relanded in https://hg.mozilla.org/integration/mozilla-inbound/rev/b4f68206c912

	Phil Ringnalda (:philor)
	Comment 27 • 11 years ago

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