Turn on thread safety assertions in opt builds on trunk.

RESOLVED FIXED in mozilla26

Status

()

Core
XPCOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: khuey, Assigned: khuey)

Tracking

unspecified
mozilla26
x86_64
Windows 7
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

2.38 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
Comment hidden (empty)

Comment 2

5 years ago
\o/
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...
Attachment #793677 - Flags: review?(bent.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/9fa41054b0dc
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26

Comment 6

5 years ago
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 )
Mmm, yes, I could see why that would be a problem.
Maybe we can gate this on --enable-profiling.
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.
Flags: needinfo?(bugs)
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.
Flags: needinfo?(bugs)

Comment 11

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

Why is profiling a build without --enable-profiling useful for you?
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

5 years ago
(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!

Updated

5 years ago
Depends on: 916851

Updated

5 years ago
Depends on: 917348
You need to log in before you can comment on or make changes to this bug.