The test TestDataStructures uses nsIntRegions which are allocated via a non-threadsafe allocator. The result is that the Test18() subroutine is not usable in a cross-thread context. Presumably nsIntRegions are also used in other contexts which will also not be usable in a cross-thread context. Making the allocator thread-local rather than global might be a possible solution.

This is one of the bugs blocking off-main-thread compositing. I don't have a good feel for how much this optimized allocator helps us. My first inclination would be to try an implementation using malloc/free and see if anything regresses.

We create and destroy regions *a lot*. I expect removing this would regress something. Maybe I'm wrong though. So, TLS recycler?

I'm looking at the allocator and I find this very concerning: http://mxr.mozilla.org/mozilla-central/source/gfx/src/nsRegion.cpp#99 This means to indicate we can have an adversarial case where a page causes a lot of region allocation. Closing the tab wouldn't release the memory. I see 3 solutions: 1) TLSing the allocator 2) Use malloc 3) Add MUTEX for the allocator Should we investigate TLSing first? Solution 2 should fix the memory poll release problem if it doesn't regress performance.

Try run for 20ae1971dd5d is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=20ae1971dd5d Results (out of 6 total builds): exception: 4 success: 2 Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-20ae1971dd5d

(In reply to Benoit Girard (:BenWa) from comment #3) > I'm looking at the allocator and I find this very concerning: > http://mxr.mozilla.org/mozilla-central/source/gfx/src/nsRegion.cpp#99 > > This means to indicate we can have an adversarial case where a page causes a > lot of region allocation. Closing the tab wouldn't release the memory. > > I see 3 solutions: > 1) TLSing the allocator > 2) Use malloc > 3) Add MUTEX for the allocator A good option might be TLS plus time-based expiration of the rect pool. Maybe someone should just write a region micro-benchmark that creates and destroys a lot of regions with 1 and 2 rectangles in them, then see if there's a measurable speedup from the free rect pool. Maybe jemalloc optimizes these 16-byte allocations really well.

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

I got a 2% tp4 regression on OSX/XP/Win7 so I think option 2 is out of the question.

That's good to know :-)

Pushed to try

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

Comment on attachment 577667 [details] [diff] [review] TLSAllocator v1 Good try run, worked well locally.

I've been playing recently with patches from bug 539356, and by default without last patches it introduces significant amount of region operations. and in couple with this fix I see significant performance hit caused by threadsafety checks on page which introduce lot of regions ops http://bubblemark.com/dhtml.htm is there are anything we can do about it?

Do we have profiles to show what is slow?

Yeah, which threadsafety checks are you referring to?

Access to per-thread rectangle memory pool.

OK, TLS access is different than thread-safety checks. I defer to comment 14 then.

http://pastebin.mozilla.org/1648703 - here is opreport with fixed 757933, nspr calls are gone, bug pthread_getspecific still on top of profile

Yay ARM. Definitely want to be __thread'ing this.

Thanks for the fix!

(In reply to Chris Jones [:cjones] [:warhammer] from comment #19) > Yay ARM. Definitely want to be __thread'ing this. Filed bug 757969 on that.