Closed Bug 401806 Opened 12 years ago Closed 12 years ago

support building with dtrace enabled on Mac OS X

Categories

(Firefox Build System :: General, enhancement)

All
macOS
enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9beta1

People

(Reporter: myk, Assigned: rflint)

Details

Attachments

(1 file, 1 obsolete file)

Now that Mac OS X 10.5 includes support for dtrace, we should support building with dtrace enabled on that OS.  According to Adam Leventhal in a blog post <http://blogs.sun.com/ahl/entry/dtrace_firefox_leopard>, it's not too tricky, and he has a patch.  Ryan Flint also has a patch <http://screwedbydesign.com/mozilla/dtracemac.diff>.
Ryan's patch looks good; just two comments:

It seems like it would be a good idea to change the polarity of the condition (if !MACOSX) since FreeBSD and other ports are, I'd argue, likely to be in the model of Solaris _and_ Mac OS X will hopefully implement -G at least as a no-op (apple bug <rdar://problem/5566030>).

I think it's worth noting in the change to the provider D file that it's a work around for apple's bugs: <rdar://problem/5194316> <rdar://problem/5565198>
Attached patch Patch (obsolete) — Splinter Review
Same patch with Adam's comments addressed.
Attachment #286883 - Flags: review?(ted.mielczarek)
Attached patch The right patchSplinter Review
Bah, It'd probably be helpful if I gave these unique names before attaching them.
Attachment #286883 - Attachment is obsolete: true
Attachment #286886 - Flags: review?(ted.mielczarek)
Attachment #286883 - Flags: review?(ted.mielczarek)
Comment on attachment 286886 [details] [diff] [review]
The right patch

I can't say I fully understand how DTrace works, but this just comments out the calls to dtrace from the build process.  What are we losing from that?
The calls eliminated are superfluous on Leopard (needed by Solaris)
Attachment #286886 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 286886 [details] [diff] [review]
The right patch

Drivers: this is a nearly zero risk patch (only changes code built with --enable-dtrace) that will allow our developers and others on Leopard to utilize DTrace - which is immensely useful for our ongoing performance efforts.
Attachment #286886 - Flags: approvalM9?
Comment on attachment 286886 [details] [diff] [review]
The right patch

a+ as we don't ship with --enable-dtrace by default (thus NPOTDB) but useful for devs anyway
Attachment #286886 - Flags: approvalM9? → approvalM9+
Assignee: nobody → rflint
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9 M9
Checking in config/rules.mk;
/cvsroot/mozilla/config/rules.mk,v  <--  rules.mk
new revision: 3.576; previous revision: 3.575
done
Checking in js/src/Makefile.in;
/cvsroot/mozilla/js/src/Makefile.in,v  <--  Makefile.in
new revision: 3.113; previous revision: 3.112
done
Checking in js/src/javascript-trace.d;
/cvsroot/mozilla/js/src/javascript-trace.d,v  <--  javascript-trace.d
new revision: 3.2; previous revision: 3.1
done
Status: NEW → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Version: unspecified → Trunk
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.