907914 - Turn on thread safety assertions in opt builds on trunk.

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Comment 2

[(no longer active)]

\o/

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 793677 [details] [diff] [review]
Patch

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

Looks good to me! I just hope we run this on b2g...

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9fa41054b0dc

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9fa41054b0dc

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

This ruins all the profiling on Nightlies.

I was profiling a testcase and wondered why AddRef/Release have suddenly become slower again
and why they end up calling PR_GetCurrentThread(). 
(The patch tripled the number of instructions in cycle collectable AddRef )

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Mmm, yes, I could see why that would be a problem.

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Maybe we can gate this on --enable-profiling.

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

Gating on --enable-profiling will work, but profiling a non-enable-profiling build (obviously) would be affected, like a true nightly (I assume they're built without --enable-profiling?)  Olli, were you profiling local builds or true nightlies?  Also, we might want to make it easier to generate --enable-profiling Try builds for similar reasons (and also publish the symbols needed if that's on).

If that's the primary issue (profiling), I'd be tempted to gate it.  As we use more complex threading, the chances of this sort of thing biting us increases.  (WebRTC, WebAudio, B2G, etc).  We purposely have left some thread-safety checks in WebRTC (and may re-enable others) for this reason, especially in non-perf-critical paths.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

I profile both. And Nightlies have --enable-profiling.
Note, for desktop development people do use debug builds and we get the
thread safety assertions in such builds.

Comment 11

[(no longer active)]

(In reply to comment #10)
> I profile both. And Nightlies have --enable-profiling.

Why is profiling a build without --enable-profiling useful for you?

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Why wouldn't it be? I may explicitly enable stuff like -fno-omit-frame-pointer.
And anyhow the point was that I profile my own builds and Nightlies, and Nightlies have
-enable-profiling set. (my own builds may or may not have that.)

Comment 13

[(no longer active)]

(In reply to comment #12)
> Why wouldn't it be? I may explicitly enable stuff like -fno-omit-frame-pointer.
> And anyhow the point was that I profile my own builds and Nightlies, and
> Nightlies have
> -enable-profiling set. (my own builds may or may not have that.)

Oh, well, the reason I asked is that -enable-profiling basically does -fno-omit-frame-pointer, but if you do that yourself then that's fine!