Closed Bug 907914 Opened 7 years ago Closed 7 years ago

Turn on thread safety assertions in opt builds on trunk.

Categories

(Core :: XPCOM, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: khuey, Assigned: khuey)

References

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
Attachment #793677 - Flags: review?(bent.mozilla)
\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
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
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)
(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.)
(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!
Depends on: 916851
Depends on: 917348
You need to log in before you can comment on or make changes to this bug.