Closed Bug 370906 (dtrace) Opened 17 years ago Closed 17 years ago

[RFE] Dynamic Tracing Framework for Mozilla

Categories

(Core :: General, defect)

Other
SunOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: john.rice, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(11 files, 36 obsolete files)

19.76 KB, text/plain
Details
2.11 KB, text/plain
Details
8.21 KB, text/plain
Details
8.74 KB, patch
ted
: review-
Details | Diff | Splinter Review
21.91 KB, patch
Details | Diff | Splinter Review
16.31 KB, patch
Details | Diff | Splinter Review
21.93 KB, patch
Details | Diff | Splinter Review
18.86 KB, patch
Details | Diff | Splinter Review
4.25 KB, patch
Details | Diff | Splinter Review
11.54 KB, patch
sayrer
: review+
Details | Diff | Splinter Review
36.93 KB, patch
Details | Diff | Splinter Review
User-Agent:       Mozilla/5.0 (X11; U; SunOS i86pc; en-US; rv:1.9a3pre) Gecko/20070219 Minefield/3.0a3pre
Build Identifier: Mozilla/5.0 (X11; U; SunOS i86pc; en-US; rv:1.9a3pre) Gecko/20070219 Minefield/3.0a3pre

The main goal for this Dynamic Tracing Framework for Mozilla is to provide a common interface for adding instrumentation points or probes to Mozilla so its behavior can be easily observed by developers and administrators even in production systems. This framework will allow Mozilla to use the appropriate monitoring/tracing facility provided by each OS. OpenSolaris, FreeBSD and MacOS will use DTrace and other OSes can use their own respective tool.

We will submit the full proposal to dev-platform@lists.mozilla.org and attach it to the bug for completeness :)

We've attached a patch which provides the basic infrastructure needed and as a proof of concept adds probes to the layout engine to track layout phase timings for paint, reflow and frame construction.

The patch was generated against Mozilla CVS head using 'cvs diff -puN'. I had to attach the mozilla-trace.d separately as I don't have commit access to Mozilla CVS to add the file.

When the patch is applied, you need to run autoconf to regenerate configure. You also need to put the attached mozilla-trace.d probe file under the top level mozilla directory. After that a standard gmake should do it.

To check you have probes built, on an OS supporting dtrace run mozilla/dist/bin/firefox-bin and type:
$ dtrace -P 'mozilla*' -l | c++filt

This will list out the probes. The proposal contains a few sample scripts to run to get the layout phase counts and timings. We've also attached a more comprehensive script that will do this and also track nested phase calls, mimics what the debug build is doing but without requiring a debug build.

We don't have a detailed knowledge of the mozilla build system and code base, so apologies in advance for any blunders we've made in the patch, but we have tested it and it all appears to work :)

John Rice
Padraig O'Briain
Alfred Peng


Reproducible: Always

Steps to Reproduce:
RFE
Attached file Mozilla probes file (obsolete) —
File specifying initial set of Mozilla probes for the layout engine. Needs to put under toplevel Mozilla dir before running autoconf and configure to get the required mozilla-trace.h file generated.
To run it just have a firefox build runnign with the dtrace probes enabled and type:

$ dtrace -qs layout_time.d -p `pgrep firefox-bin` -o out.txt
$ cat out.txt | c++filt
Measure the new AutoLayout Phases (Paint, Reflow and Frame Construction) for Firefox when loading a page, using the Mozilla dtrace layout probes, layout-start and layout-end:

Usage: dtrace -qs layout_phaseflow.d -p <firefox-pid> <time (sec) to run> | c++filt

Example: dtrace -qs layout_phaseflow.d -p `pgrep firefox-bin` 20 | c++filt
Will post this on dev-platform@lists.mozilla.org, just wanted it with the bug so we had everything in the one place.
Assignee: download-manager → nobody
Status: UNCONFIRMED → NEW
Component: Download Manager → General
Ever confirmed: true
Product: Mozilla Application Suite → Core
QA Contact: general
Version: unspecified → Trunk
Comment on attachment 255689 [details] [diff] [review]
Mozilla Dynamic Tracing Framework + layout probes patch

>Index: mozilla/config/Makefile.in
no.

this should work:
+ifdef HAVE_DTRACE
+HEADERS += \
+      $(DEPTH)/mozilla-trace.h \
+      $(NULL)
+endif

>+MOZILLA_DTRACE_SRC = $(DEPTH)/mozilla-trace.d

hrm.

>+	dtrace -G -C -32 -s $(MOZILLA_DTRACE_SRC) -o $(DTRACE_PROBE_OBJ) $(OBJS)

Can you include a url reference explaining the args here? (just a comment, not for the patch, that way people reading cvs blame can get to this bug and the comment.)

no
>+ifdef DTRACE_PROBE_OBJ
>+$(LIBRARY): $(OBJS) $(DTRACE_PROBE_OBJ) $(LOBJS) $(SHARED_LIBRARY_LIBS) $(EXTRA_DEPS) Makefile Makefile.in

EXTRA_DEPS += $(DTRACE_PROBE_OBJ)

I think.

sucky :(
>+ifdef DTRACE_PROBE_OBJ
>+	$(AR) $(AR_FLAGS) $(OBJS) $(DTRACE_PROBE_OBJ) $(LOBJS) $(SUB_LOBJS)
>+else
> 	$(AR) $(AR_FLAGS) $(OBJS) $(LOBJS) $(SUB_LOBJS)
>+endif

I'd vote for EXTRA_DEPS being used here.

>Index: mozilla/layout/Makefile.in
>+DTRACE_PROBE_OBJ = $(LIBRARY_NAME)-dtrace.$(OBJ_SUFFIX)

>Index: mozilla/layout/base/Makefile.in
>+DTRACE_PROBE_OBJ = $(LIBRARY_NAME)-dtrace.$(OBJ_SUFFIX)

kinda pondering that you have this more than once...
rules?

>Index: mozilla/layout/base/nsCSSFrameConstructor.cpp
>+#ifdef HAVE_SYS_SDT_H
>+#include "mozilla-trace.h"
>+#endif

> nsCSSFrameConstructor::ConstructRootFrame(nsIContent*     aDocElement,
>                                           nsIFrame**      aNewFrame)
>+  MOZILLA_LAYOUT_START((int *)mPresShell->GetPresContext(), eLayoutPhase_FrameC);

Should probably have dbaron or bz ponder the name.

Is this reentrant friendly?

>+  MOZILLA_LAYOUT_END((int *)mPresShell->GetPresContext(), eLayoutPhase_FrameC);

> nsCSSFrameConstructor::ReconstructDocElementHierarchy()

I'd rather two ifdefs with a single common code path in the middle.
  nsresult rv;
#ifdef HAVE_SYS_SDT_H
  MOZILLA_LAYOUT_START((int *)mPresShell->GetPresContext(), eLayoutPhase_FrameC);
#endif
  AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC);
  rv = ReconstructDocElementHierarchyInternal();
#ifdef HAVE_SYS_SDT_H
  MOZILLA_LAYOUT_END((int *)mPresShell->GetPresContext(), eLayoutPhase_FrameC);
#endif
  return rv;

note the cannonical "rv" (not res).

>Index: mozilla/layout/base/nsPresContext.h
>@@ -133,6 +133,17 @@ enum nsLayoutPhase {

change:
  #ifdef DEBUG
   ...
>+#else
>+#ifdef HAVE_SYS_SDT_H
 + ...
 +#endif

to:
#if defined(DEBUG) || defined(HAVE_SYS_SDT_H)

>Index: mozilla/layout/base/nsPresShell.cpp
>@@ -22,7 +22,7 @@
>  *
>  * Contributor(s):
>  *   Steve Clark <buster@netscape.com>
>- *   H�kan Waara <hwaara@chello.se>
>+ *   H\345kan Waara <hwaara@chello.se>

Your editor is javaish, you might need to hand edit this out for the future :(.

I'd vote for losing the comment:
>+// Mozilla Trace

>+#ifdef HAVE_SYS_SDT_H

And I wonder if we can change this to a single statement

INCLUDE_MOZILLA_DTRACE

or something (and have mozilla-config.h make that a useful statement or nop)

just a thought. I'm not the final word (on this or anything else).

As for new files.

please use cvsdo add <http://viper.haque.net/~timeless/redbean/>

cvsdo add files ....
cvs diff -pNU8

generate a new patch, attach it and obsolete all the old attachments :)
Attachment #255689 - Flags: superreview?(dbaron)
Attachment #255689 - Flags: review?(bzbarsky)
Attachment #255691 - Attachment mime type: text/x-dsrc → text/plain
Attachment #255694 - Attachment mime type: text/x-dsrc → text/plain
Summary: [RFE] Dynamic Tracing Frmework for Mozilla → [RFE] Dynamic Tracing Framework for Mozilla
Comment on attachment 255689 [details] [diff] [review]
Mozilla Dynamic Tracing Framework + layout probes patch

I'm not going to be a position to review anything like this until at least mid-April, possibly much longer.  Please ask someone else.
Attachment #255689 - Flags: review?(bzbarsky) → review?
Comment on attachment 255689 [details] [diff] [review]
Mozilla Dynamic Tracing Framework + layout probes patch

I don't see why you're putting code exactly at the points exactly at the callers of existing code that you can hook into (but in a less reliable way, since you don't get the benefits of the guard object's destructor always running).  review- based on that alone.

I'd also warn you that there are a number of other things that happen lazily within these phases, such as style resolution, so any performance numbers you get out of just this instrumentation are highly questionable.  I'm also not sure why you need instrumentation at all to determine general amounts of time spent in different areas of the program.  We've been doing splitting with jprof without any intrusive edits to the code.  (See mozilla/tools/jprof/split-profile.pl   For an example of its use, see bug 144533 comment 40.)

Also, some of these hooks are at points that run a *lot*.  And it would be significantly worse if we were actually doing correct instrumentation to distinguish style resolution.  How much runtime overhead would adding these to release builds cause?

Also, what's the goal of providing this to end users?  Is it to have those users file useful performance bugs?  If so, we probably do need accurate distinctions of the phases, or the end users will just assume that the tool is right and constantly correct developers when they correct what the tool is saying.
Attachment #255689 - Flags: superreview?(dbaron)
Attachment #255689 - Flags: review?
Attachment #255689 - Flags: review-
David - we just took the Auto Layout as example suggested by Robert O'Callahan, who wanted to get some data on the phase timings. If there are other endpoints we need to watch then we should do so, such as the style resolution.

The probes have no overhead if they are not enabled by running dtrace. That's the beauty of them. If a user has an issue they can just run a script there and then to generate data off a non-debug build on a system with dtrace.
No overhead?  Do they compile to something that's in a different section of the shared library that doesn't even get loaded if you're not running dtrace?  How do you manage that?
David to get into the gory details you need some of the dtrace kernel hackers to explain, but I think it involves the kernel linker putting in no ops for dtrace function names with well defined _dtrace_probe prefix when they are not enabled. I can ping them if you want.

In addition if there is any overhead in setting up a probes arguments the dtrace header file generated from the probes file contains IS_ENABLED macros that can be used to bracket the probes and reduce the over head to virtually nil [this is the technique used in Ruby]
http://blogs.sun.com/ahl/entry/user_land_tracing_gets_better

More details on probe effect at:
http://www.sun.com/bigadmin/content/dtrace/dtrace_usenix.pdf
4.2 Statically-defined Tracing
"In keeping with our philosophy of zero probe ef-         
fect when disabled, we have implemented a statically-    
defined tracing (SDT) provider by defining a C macro       
that expands to a call to a non-existent function with   
a well-defined prefix (“ dtrace probe ”). When the         
kernel linker sees a relocation against a function with  
this prefix, it replaces the call instruction with a no-
operation and records the full name of the bogus func-
tion along with the location of the call site. When the
SDT provider loads, it queries this auxiliary structure
and creates a probe with a name specified by the func-
tion name. When an SDT probe is enabled, the no-
operation at the call site is patched to be a call into
an SDT-controlled trampoline that transfers control into
DTrace."
timeless - just responding to a few of your comments above:

We'll rework the patch as you suggested and resubmit:
1/ We'll create a simplified dtrace -h script or c program that can run on a linux system and generate the mozilla-trace.h from the mozilla-trace.d file. This will allow us to have both files in cvs and the build autobot can use the script to regenerate the  header if the probes file has changed as you suggested. Always having the header will allow us to remove a lot of the guard defines in the code, the macros will just become noops if dtrace is not on the system.
2/ You asked above if the probes are reentrant - each probe is atomic and so this issue does not arise, there is no dependency between each probe.
3/ On the dtrace -G:
We'll put in the patch comment with the patch when we regenerate it.
dtrace -G -C -32 -s $(MOZILLA_DTRACE_SRC) -o $(DTRACE_PROBE_OBJ) $(OBJS)

For the terminally curious:
USDT Probes: http://docs.sun.com/app/docs/doc/817-6223/6mlkidlms?a=view

-G: tells dtrace to process the list of .o's looking for USDT probes, change their status to IGNORE and generate a special <probe>.o that you should link against so the kernel linker can correctly initialized things when the shared lib is loaded.
-C: use the C preprocessor to process the specified .d file, allows you to have #includes
-32: compile for 32 bit architecture, we need to change what I gave you so it will work on 64 bit arch as well :(
-s: source of probes, mozilla-trace.d in this instance
-o: output special <probe>.o file, call it anything you want, just need to link against it.
timeless:

I have a query about one of your comments in #6
sucky :(
>+ifdef DTRACE_PROBE_OBJ
>+	$(AR) $(AR_FLAGS) $(OBJS) $(DTRACE_PROBE_OBJ) $(LOBJS) $(SUB_LOBJS)
>+else
> 	$(AR) $(AR_FLAGS) $(OBJS) $(LOBJS) $(SUB_LOBJS)
>+endif

I'd vote for EXTRA_DEPS being used here.

Is this suggestion safe as EXTRA_DEPS is currently defined in a number of Makefiles and using $(EXTRA_DEPS) instead of $(DTRACE_PROBE_OBJ) could result in  extra files, which are not object files being added to the library archive?
(In reply to comment #12)
timeless - we where thinking about the simplified dtrace -h for the build autobot:
 
> 1/ We'll create a simplified dtrace -h script or c program that can run on a ... 

It looks like it would be a lot of work to pull this out of the dtrace code [it compiles the probe during this -h processing]. What might make a lot more sense is just to check the mozilla-trace.h into cvs. Means its always there for the builds and the trace macros are compiled out if dtrace is not on the system.
If someone wants to add new probes they do so on a system with dtrace support, modify the mozilla-trace.d, rerun the configure which will recreate the mozilla-trace.h and either create a patch or check it into to cvs.

What do you think?
Updated to reflect changes in implementation after feedback from timeless
Attachment #255695 - Attachment is obsolete: true
Updated to reflect changes in implementation after feedback from timeless

Main differences from initial patch:

configure.in:
-------------
Disable dtrace by default. Add --enable-dtrace option to configure.in which needs to be specified for dtrace support to be included even on OS's that support dtrace.

mozill-trace.h.in:
-------------------
Assume mozilla-trace.h.in is under cvs and can be used to generate mozilla-trace.h with configure on all architectures.

Simplifies probe includes, macros do not need to be guarded with #ifedf's as they will always compile out to noops on architectures without dtrace or if the --enable-dtrace option has not been passed to configure.
In C++ files now just add probe header and macro:
#include "mozilla-trace.h"
	:
nsCSSFrameConstructor::ConstructRootFrame(...)
 {
  TRACE_MOZILLA_LAYOUT_START(...);

mozilla-trace.h (generated)
----------------------------
Added INCLUDE_MOZILLA_DTRACE to mozilla_dtrace.h which is defined if dtrace support is enabled and can be used if certain setup code needs added for a given dtrace probe in a component header for instance.

mozilla-trace.d:
----------------
Changed namespace of probes:
mozilla -> trace_mozilla
Probe macros now have the form of TRACE_MOZILLA_<SUBCOMPONENT_NAME>(...)


config/rules.mk:
-----------------
+$(DTRACE_PROBE_OBJ): $(OBJS)
+	dtrace -G -C -32 -s $(MOZILLA_DTRACE_SRC) -o $(DTRACE_PROBE_OBJ) $(OBJS)

Generates required probe.o to link against:
-G: Generate an ELF file containing an embedded DTrace program. The DTrace probes specified in the program are saved inside of a relocatable ELF object which can be linked into another program. 
-C: Run the C preprocessor cpp(1) over D programs before compiling them. 
-32/64: determine the ELF file format (ELF32 or ELF64) produced by the -G option. This is meant to be auto detected, may have to add a DTRACEOPTIONS_FLAG to configure to allow this to be specified at build time.
-s: Compile the specified D program source file.
-o: the ELF file generated with the -G option is saved using the pathname specified as the argument for this operand. 
For explanation of dtrace params, see dtrace man page:
http://docs.sun.com/app/docs/doc/816-5166/6mbb1kq15?a=view#indexterm-157 

To get dtrace probe.o file picked up when building Shared Libraries need to add this in several places in rules.mk.
EXTRA_DEPS, $(AR), $(MKSHLIB)

layout/base/Makefile.in:
-------------------------
Changes in rules.mk allow us to include DTRACE_PROBE_OBJ in base makefile:
layout/base/Makefile.in
No change in layout/Makefile.in required.

layout/base/nsPresContext.h
----------------------------
Using:
#if defined (DEBUG) || defined (INCLUDE_MOZILLA_DTRACE)
to include layout phase enum if dtrace is enabled

mozilla-trace.d
----------------
Includes mozilla-trace.d so need to have this as separate attachment [cvsdo is my friend]
Attachment #255689 - Attachment is obsolete: true
Attachment #255690 - Attachment is obsolete: true
Change in probe namespace, trace_mozilla, script updated to work with new probes
Attachment #255691 - Attachment is obsolete: true
Change in probe namespace, trace_mozilla, script updated to work with new probes
Attachment #255694 - Attachment is obsolete: true
Updated the proposal to reflect the changes in the framework and layout patch and the addition of the javascript probes patch [see patch comments].
Attachment #256048 - Attachment is obsolete: true
Update to framework and layout patch:
Given David Baron's comments on problems with having matching exit probes in a large function block with an initial entry probe.

As a workaround we declare a struct with a constructor and destructor into which you put the probes and you just declare an instance of this struct where you would have put the entry probe. When this struct goes out of scope it's destructor is called and the exit probe fires. There is no need to add a separate exit probe explicitly in the code. This does mean you pay the cost of declaring an extra struct in the calling code, but this should be neglible as the stuct has no expensive initialisation code, it just stores the construction parameters.

We made these changes for the layout probes. This change means that the Function reported for these probes would now be the constructor and destructor of the struct, so we wrapped them in more meaningful extern C trace function names [refer to nsPresContext.h, nsPresContext.cpp in the patch].
Attachment #256051 - Attachment is obsolete: true
Updated the script to reflect changes in the layout probes.
Attachment #256052 - Attachment is obsolete: true
Updated the script to reflect changes in the layout probes.
Attachment #256055 - Attachment is obsolete: true
Attached patch Javascript probes patch (obsolete) — Splinter Review
This is an incremental patch that can be applied on top of the framework and layout probes patch.

It provides Brendan Gregg's mozilla javascript probes as described at:
http://blogs.sun.com/brendan/entry/dtrace_meets_javascript
And demo'ed at: http://frsun.downloads.edgesuite.net/sun/07C00953/index.html

All of the probes listed in Brendan's Blog on JavaScript and DTrace will work, but we have changed the namespace to be in sync with the other mozilla probes, so you will need to change the probe names in the script s appriopriately, for example:
  
Change: 
javascript*:::function-entry -> trace_mozilla*:::js_function-entry

To list available probes once once the patch is applied and mozilla is rebuilt with --enable-dtrace, just type:

$ dtrace -n 'trace_mozilla*:::js*' -l
Updated to apply to CVS HEAD
Attachment #259787 - Attachment is obsolete: true
Attached patch Javascript probes patch (obsolete) — Splinter Review
Update to apply to CVS HEAD and fix some errors
The following simple dtrace scripts will give a quick test of the javascript functionality:

Trace all javascript functions and print out the file they have been called from, the class name and function name. We use the flow indent flag -F to allow the entry and return function calls to be matched up and appropriately indented.
dtrace -F -n 'trace_mozilla22094:::js_function*{printf("%s %s->%s", basename(copyinstr(arg0)), copyinstr(arg1), copyinstr(arg2));}'

Note: often these last two params are not set and just return null, particularly on the internal chrome js calls. This will need to be addressed in the future.

Simple test for object lifecycle probes:
dtrace -n 'trace_mozilla22094:::js_object*{trace(arg0);}' 
Update to apply to the CVS head.
Attachment #257960 - Attachment is obsolete: true
Following is a proposal raised by Brendan Gregg. Hope to get some feedback from you guys here.

"
The DTrace patches currently provide this,

	trace_mozilla:::layout*
	trace_mozilla:::js_*

we think that this approach is better (of course, your opinion counts as well),

	mozilla:::layout*
	javascript:::*

(we do get fussy over conventions, as it would be nice if different providers looked similar).

Reasons for each (and you are welcome to point out problems with this):

mozilla:::layout* - "mozilla" won't clash with anything else - use it! We want to tell customers "this is the mozilla provider", not "this is the mozilla provider called trace_mozilla". And if other people write their own mozilla providers for different versions? No problem - so long as they export the same layout* probes. As a user writing a DTrace script, I should be able to write stable mozilla::: scripts that run on different mozilla versions, because they are exporting the same interface.

javascript:::* - because it is the javascript provider, period. What about opera and other browsers? Since the javascript provider will be stable, they can export to the exact same namespace with the exact same probes. So long as the javascript provider exports no mozilla implementation specifics, which I don't think it does.

The mental connection I had to make, was that entirerly different engines (mozilla versions, firefox/opera), can export the same stable provider and be used in parallel on the same system. Sounds a bit frightening, but it works fine.

Bryan Cantrill just ran the following script with ruby running,

	# dtrace -n 'function-entry { trace(copyinstr(arg0)); }'

and was suprised to see results that looked like javascript. He was running the old javascript bits, and his probe was matching crazy,

	javascript*:::function-entry
	ruby*:::function-entry

crazy! different implementations, same probename and args. but it emphasised that this is doable.
"
As the patch contains the code change to the build system, cc benjamin to get some suggestions.
If you want me to review something, please set the appropriate flag.
Attached patch JavaScript probes patch v2 (obsolete) — Splinter Review
I've attached a patch for the latest version of the JavaScript probes.
The new additions are,

   * the ability to identify anonymous functions (js_function-info)
   * access to function entry arguments (js_function-args)
   * access to function return value (js_function-rval)

I was finding that the JavaScript provider was only helping with about
4 problems out of 10 (still, better than nothing). The improvements that
I added should take that to around 8 out of 10 - so it is now proving to
be really useful.

Anyhow, here are some before and after screenshots,                             
                                                                                
Old,

# ./js_funcflow.d
C TIME                  FILE                     -- FUNC
1 2007 May 31 06:07:13  clock2.html              -> startTime
1 2007 May 31 06:07:13  clock2.html                -> getHours
1 2007 May 31 06:07:13  clock2.html                <- getHours
1 2007 May 31 06:07:13  clocklib.js                -> padLibZero
1 2007 May 31 06:07:13  clocklib.js                <- padLibZero
1 2007 May 31 06:07:13  clock2.html                -> getMinutes
1 2007 May 31 06:07:13  clock2.html                <- getMinutes
1 2007 May 31 06:07:13  clocklib.js                -> padLibZero
1 2007 May 31 06:07:13  clocklib.js                <- padLibZero
1 2007 May 31 06:07:13  clock2.html                -> getSeconds
1 2007 May 31 06:07:13  clock2.html                <- getSeconds
1 2007 May 31 06:07:13  clocklib.js                -> padLibZero
1 2007 May 31 06:07:13  clocklib.js                <- padLibZero
1 2007 May 31 06:07:13  clock2.html                -> getElementById
1 2007 May 31 06:07:13  clock2.html                <- getElementById
1 2007 May 31 06:07:13  clock2.html                -> setTimeout
1 2007 May 31 06:07:13  clock2.html                <- setTimeout
1 2007 May 31 06:07:13  clock2.html              <- startTime
^C

New,

# ./js_argflow.d
C TIME                  FILE                 -- FUNC()
1 2007 May 31 06:07:34  clock2.html           -> startTime()
1 2007 May 31 06:07:34  clock2.html             -> getHours()
1 2007 May 31 06:07:34  clock2.html             <- startTime:9 = 0x80000000
1 2007 May 31 06:07:34  clocklib.js             -> padLibZero(0x6)
1 2007 May 31 06:07:34  clocklib.js             <- padLibZero:3 = 0x955fd70
1 2007 May 31 06:07:34  clock2.html             -> getMinutes()
1 2007 May 31 06:07:34  clock2.html             <- startTime:10 = 0x80000000
1 2007 May 31 06:07:34  clocklib.js             -> padLibZero(0x7)
1 2007 May 31 06:07:34  clocklib.js             <- padLibZero:3 = 0x955fd50
1 2007 May 31 06:07:34  clock2.html             -> getSeconds()
1 2007 May 31 06:07:34  clock2.html             <- startTime:11 = 0x80000000
1 2007 May 31 06:07:34  clocklib.js             -> padLibZero(0x22)
1 2007 May 31 06:07:34  clocklib.js             <- padLibZero:4 = 0x22
1 2007 May 31 06:07:34  clock2.html             -> getElementById()
1 2007 May 31 06:07:34  clock2.html             <- startTime:12 = 0x80000000
1 2007 May 31 06:07:34  clock2.html             -> setTimeout()
1 2007 May 31 06:07:34  clock2.html             <- startTime:13 = 0x80000000
1 2007 May 31 06:07:34  clock2.html           <- startTime:13 = 0x80000000
^C


Old,

# ./js_funccalls.d
Tracing... Hit Ctrl-C to end.
^C
 FILE                             FUNC                                    CALLS
 clock2.html                      getElementById                              1
 clock2.html                      getHours                                    1
 clock2.html                      getMinutes                                  1
 clock2.html                      getSeconds                                  1
 clock2.html                      setTimeout                                  1
 clock2.html                      startTime                                   1
 clocklib.js                      padLibZero                                  3

New,

# ./js_funcinfo.d
Tracing... Hit Ctrl-C to end.
^C
 BASE[FILE:LINE]         FUNCNAME             RUN[FILE:LINE]             CALLS
 clock2.html:7           getElementById       clock2.html:12                 1
 clock2.html:7           getHours             clock2.html:9                  1
 clock2.html:7           getMinutes           clock2.html:10                 1
 clock2.html:7           getSeconds           clock2.html:11                 1
 clock2.html:7           setTimeout           clock2.html:13                 1
 clock2.html:7           startTime            clock2.html:13                 1
 clocklib.js:2           padLibZero           clock2.html:10                 1
 clocklib.js:2           padLibZero           clock2.html:11                 1
 clocklib.js:2           padLibZero           clock2.html:9                  1


To take the provider further (and to solve 10 out of 10 problems), may
require some moderate code changes to libmozjs for the following features,

        * stack traces (perhaps by integrating with jsd)

        * objects as debug strings (more jsd integration?)

And so these may be best for version 3 of this provider - which would be after the existing JavaScript probes are integrated (assuming that they ever are). I would also welcome a Mozilla developer who is more familiar with js/src and js/jsd to take ownership of the JavaScript provider and to build on the work I've done so far.

Lastly, I am really hoping that we can call these probes "javascript:::*" and not "trace_mozilla:::js_*" - as this is the "JavaScript" provider. It would require hand generation of the header files rather than "dtrace -h", so that the code macros can remain of the style "TRACE_MOZILLA_JS_FUNCTION_ENTRY" (which is perfectly fine). I think the initial change from "javascript:::*" to "trace_mozilla:::js_*" was a side effect of changing the macros, but a side effect that we can avoid by not auto generating mozilla-trace.h.
Attachment #259788 - Attachment is obsolete: true
Split the “Mozilla Dynamic Tracing Framework + layout probes patch” into two separated ones. This one is for the DTrace Framework and ask for review.

Please notice that I'm still using the provider name "trace_mozilla" and no probes are defined in this patch. It will have no impact on the code if --enable-dtrace is not passed to configure.
Attachment #257963 - Attachment is obsolete: true
Attachment #267801 - Flags: review?(benjamin)
John Rice demoed the Mozilla DTrace to jst and schrep when they visited Sun Beijing office. cc jst into this bug.

DTrace will be provided in Leopard (MacOS 10.5). We'll provide the VMWare/ parallel images for Solaris with some DTrace tools soon. For more information about Mozilla DTrace, please go to the Mozilla DTrace project in OpenSolaris community: http://opensolaris.org/os/project/mozilla-dtrace/.
Update the framework patch to reflect the provider name change, from "trace_mozilla:::*" to "mozilla:::*" as Brendan Gregg suggested.

No real probes defined in this patch. The layout probes and the javascript probes can be added based on this framework.
Attachment #267801 - Attachment is obsolete: true
Attachment #268059 - Flags: review?(benjamin)
Attachment #267801 - Flags: review?(benjamin)
Attachment #268059 - Flags: review?(benjamin) → review?(ted.mielczarek)
I have been trying for a while now, what do I have to do to get this to apply to head (the newer V2 javascript stuff) 

At the mo I am stuck with the older patches, since these are the only ones that I can get to tastefully apply
Update to apply to trunk code
Attachment #265808 - Attachment is obsolete: true
One line update to the patch to apply to trunk code.

Greg, if you want to try out the javascript probes, please patch the framework + layout first and then this one.

BTW, there are other two approaches to try the probes:
1. Solaris VMware images are available for download: http://wiki.mozilla.org/SolarisVM. And the Firefox 3.0 with DTrace has been bundled with it.
2. If you have already installed Solaris on your box, the Firefox 3.0a3 with DTrace is available here: http://www.opensolaris.org/os/project/mozilla-dtrace/.
Attachment #266840 - Attachment is obsolete: true
My interest is to try it with xulrunner, however I will do a firefox trial first 
Updated v5 to apply to CVS HEAD
Blocks: jsdtrace
The previous layout probes patch no longer works with CVS HEAD. The layout probes are in the component gklayout. This used to be in the shared library components/libgklayout.so. In CVS HEAD the components are in the shared library toolkit/library/libxul.so. A library archive is created for each component and these library archives are linked into libxul.so.

To get the probes to work I need to call dtrace -G when creating the shared
library and specify the the object files which contain dtrace calls. This will be necessary if we put probes into more than one of the library archives which make up libxul.so.

I can do this by specifying the library archives containing dtrace probes in the variable MOZILLA_PROBE_LIBS in toolkit/library/Makefile.in.
Then in config/rules.mk when building a shared library I extract the object files from the list of library archives in MOZILLA_PROBE_LIBS and run dtrace -G on them.

However, I have a problem with the link line. I need to specify the objects 
which have been processed by dtrace -G on the link line. I can do this by
specifying $(PROBE_LOBJS) in the link line. I do not want to specify the library archives that these objects came from on the link line.
The list of the archives is specified in SHARED_LIBRARY_LIBS and the archives
which I do not want to specify on the link line is in MOZILLA_PROBE_LIBS.
My solution is to remove all the objects from the library archives in
MOZILLA_PROBE_LIBS after running dtrace -G. When the library archive is 
specified on the link line it is empty. After the linking the shared library 
the library archives in MOZILLA_PROBE_LIBS are deleted.

An alternative which I looked at was to extract the object files from
all the library archives and specify these objects on the link line
instead of the library archives. This did not work because a file called
nsWildCard.o is in more than one archive and the files are different.
FWIW, the library that the layout code lives in depends on whether libxul is enabled when building. If it's not enabled, the code will still be in gklayout, if not, it'll be in the xul library.
This patch replaces v8 and is updated to apply to CVS HEAD as of this morning. I also made a change in config/rules.mk so that the dtrace object file is built for the JavaScript dtrace probes.
Attachment #274169 - Attachment is obsolete: true
The updated patch will cause layout probes to be generated whether or not libxul is enabled. I test the case where libxul is not enabled by configuring with --enable-debug.
Attachment #276949 - Attachment is obsolete: true
Update patch to apply to CVS HEAD.
Attachment #277087 - Attachment is obsolete: true
This attachment and the next one splits the patch into two parts. This part is the framework part and is a prerequisite for the JavaScript probes patch in issue 388564.
Attached patch layout probes vc (obsolete) — Splinter Review
This patch and Mozilla Dynamic Tracing Framework v3 patch replaces Mozilla Dynamic Tracing Framework + layout probes vb patch.

The patch Mozilla Dynamic Tracing Framework v3 patch is a prerequisite for this patch.
Attachment #277370 - Attachment is obsolete: true
Updated patch so that it applies cleanly
Attachment #277393 - Attachment is obsolete: true
Attached patch layout probes vd (obsolete) — Splinter Review
updated patch to apply to CVS HEAD.
Attachment #277395 - Attachment is obsolete: true
update patch to apply cleanly to HEAD.
Attachment #278054 - Attachment is obsolete: true
Mozilla Dynamic Tracing Framework v6

Removing layout probes and adding some loadURI start and done probes
Attachment #278998 - Attachment is obsolete: true
Mozilla Dynamic Tracing Load URI probes v1

Adding load URI start and done probes.

http://blogs.sun.com/jmr/entry/adding_custom_load_url_probes

$ dtrace -ln "moz*:::"
   ID   PROVIDER            MODULE                          FUNCTION NAME
71986 mozilla18214         libxul.so          mozdtrace_load_uri_start load-uri-start
71987 mozilla18214         libxul.so          mozdtrace_load_uri_done load-uri-done

$ dtrace -qn moz*:::'{printf("%s\n",copyinstr(arg0));}'
http://www.mozilla.org/
Attachment #278055 - Attachment is obsolete: true
Changes to allow debug build and adding Channel param to give probes unique ID.
Attachment #279165 - Attachment is obsolete: true
Adding load Image start probe. Need to get a unique ID, passing in a mRequest which is giving me 0 back. If anyone can suggest something in the loadImage code to use as a unique ID that would be great. Also not sure where to add the load Image Done probes. So if someone can point out where to add the Image Done probes just let me know and I can add them.
Attachment #279166 - Attachment is obsolete: true
Johnny I was talking with Brendan about these probes and we think it makes more sense to have a general load-start and load-done set of probes with a suitable enum param to identify the type of load that is taking place. I'd then like to add a payload to the probe of the URI data as a struct with simple strings. This will only need to be constructed when the probes are enabled. Having it built will make the scripts a lot easier to use, rather than having to do the URI processing in the scripts.

Current probes:
loadURI-start(int UNIQUE_ID, char * URI)
loadURI-done(int UNIQUE_ID, char * URI)
loadImage-start(int UNIQUE_ID, char * URI)
loadImage-done(int UNIQUE_ID, char * URI)

Proposed probes:

load-start ( int UNIQUE_ID, int LOAD_TYPE, int * LOAD_DATA)
load-done ( int UNIQUE_ID, int LOAD_TYPE, int * LOAD_DATA)

Purpose of the UNIQUE_ID is to allow you to match async load-start and load-done probes.

LOAD_TYPE
===========
enum nsTraceLoadType {
eLoad_URI,
eLoad_Image
};

LOAD_DATA
==============
struct nsTraceLoadData {
// From nsIChannel
char * contentType  // Content Type

// From nsIURI
char * spec               // Complete URI

// <scheme>://<username>:<password>@<hostname>:<port>//<directory><basename>.<extension>;<param>?<query>#<ref>

char * scheme          // Protocol to which this URI refers
char * username
char * password      
char * host                // Internet domain name
char * port
char * path               // <filepath>;<param>?<query>#<ref>

// From nsIURL
char * filePath         // <directory><basename>.<extension>
char * fileName       // <basename>
char * fileExtension // <extension>      
char * param
char * query
char * ref
};

Struct elements will be set to empty string if element is not applicable to this URI.

Refer to:
http://lxr.mozilla.org/mozilla/source/netwerk/base/public/nsIURI.idl
http://lxr.mozilla.org/mozilla/source/netwerk/base/public/nsIURL.idl
http://lxr.mozilla.org/mozilla/source/netwerk/base/public/nsIChannel.idl

Once we get these ones in then we need to look at adding probes for the other stages of loading and displaying a web page:

LoadURI       LoadImage
   |            |
    ------------
         |
      Network --> DNS Request
         |
      Parser
         |
      Content DOM
         |
      Layout
         | 
      Painting
Adding common load-start and load-done probes for URI and Image laoding
Attachment #279199 - Attachment is obsolete: true
Adding common load-start and load-done probes for URI and Image loading. For example output and scripts using the probes refer to:

Sample Image Loading Stats:
http://blogs.sun.com/jmr/resource/browser_time_image3.txt
Script used to generate Stats:
http://blogs.sun.com/jmr/resource/browserspy_time_image3.d

Will move onto DNS lookup probes next:
dnslookup-start: nsHostResolver::ResolveHost(...) and 
dnslookup-done: nsHostResolver::OnLookupComplete(...) 
 	 	
I posted up on moz.dev.performance to get some feedback on other probe points:
"Performance probes - looking for good start/ done points"

Boris Zbarsky has pointed me at some spots for painting and layout. I'll perhaps combine this with the earlier layout probes. Dave (dbaron) if you have any suggestions please holler, I definitely need the input :)

Jonas Sicking has given me a few hints on the DOM creation as well. So we are making progress i think.
Attachment #279200 - Attachment is obsolete: true
Update to apply cleanly to CVS head and fix some errors
Attachment #281294 - Attachment is obsolete: true
Created combined URI and Image load timing script, available at:

Script: http://blogs.sun.com/jmr/resource/browserspy_time_URI_image.d
Sample output: http://blogs.sun.com/jmr/resource/browserspy_time_URI_image.txt
Updating uri and image load probes to have a separate load-inage-init probe.
Adding dnslookup init, start and done probes:
Refer to:
http://blogs.sun.com/jmr/entry/dtrace_mozilla_what_s_going
Attachment #281320 - Attachment is obsolete: true
Updating uri and image load probes to have a separate load-inage-init probe.
Adding dnslookup init, start and done probes:
Refer to:
http://blogs.sun.com/jmr/entry/dtrace_mozilla_what_s_going
Attachment #281297 - Attachment is obsolete: true
Sorry, this has been sitting in my review queue way too long.  I'll take a look at it tomorrow.
Comment on attachment 268059 [details] [diff] [review]
 Mozilla Dynamic Tracing Framework v2

>Index: configure.in
>===================================================================
>
>+AC_ARG_ENABLE(dtrace,
>+              "build with dtrace support if available [default=no]",
>+              [enable_dtrace="yes"],)
>+if test "x$enable_dtrace" = "xyes"; then
>+  AC_CHECK_HEADER(sys/sdt.h, HAVE_DTRACE=1; dtrace -h -s mozilla-trace.d -o mozilla-trace.h.in)
>+  if test -n "$HAVE_DTRACE"; then
>+      AC_DEFINE(INCLUDE_MOZILLA_DTRACE)
>+  fi
>+fi

Enable arguments should error if the requirements aren't met, not silently disable themselves.  You could also optionally enable this by default on Solaris.  See for example: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/configure.in&rev=1.1882&mark=5335-5340#5335


>Index: mozilla-trace.d
>Index: mozilla-trace.h.in

I don't want these files in the root directory.  Can we find a better place for them to live?  Even somewhere in build/ would be more agreeable to me.  Additionally, if mozilla-trace.h.in is a generated file, why are we checking in a copy of it?  Is this just so you can unconditionally include it later?


>Index: config/Makefile.in
>===================================================================
> HEADERS = \
> 	nsBuildID.h \
> 	$(DEPTH)/mozilla-config.h \
>+	$(DEPTH)/mozilla-trace.h \
> 	$(srcdir)/nsStaticComponents.h \
> 	$(NULL)

If we don't checkin the generated header file, you could wrap this line in 
ifdef HAVE_DTRACE
endif


>Index: config/rules.mk
>===================================================================
>+ifdef HAVE_DTRACE
>+ifdef DTRACE_PROBE_OBJ
>+ifndef MOZILLA_DTRACE_SRC
>+MOZILLA_DTRACE_SRC = $(DEPTH)/mozilla-trace.d
>+endif
>+$(DTRACE_PROBE_OBJ): $(OBJS)
>+	dtrace -G -C -32 -s $(MOZILLA_DTRACE_SRC) -o $(DTRACE_PROBE_OBJ) $(OBJS)
>+endif
>+endif

Reading your other patch, is there any reason we need to specify DTRACE_PROBE_OBJ in the Makefiles, or could we just have a variable like USE_DTRACE=1, and then autogenerate a probe object filename from LIBRARY_NAME or MODULE or whatever in here?

I notice there's a newer version of this patch, but maybe you can roll my review comments into that and post a refreshed version?  I promise you a speedier review this time!
Attachment #268059 - Flags: review?(ted.mielczarek) → review-
On choice of directory, when we were talking about probes in general a while back, Vlad suggested making a dedicated "probes" directory. It could be toplevel, since probing machinery doesn't quite fit into other categories, or perhaps go under "modules".

Also, I bumped into a compiler error on Leopard with the latest patch version; in nsHostResolver.cpp:mozdtrace_dnslookup_request_done, struct nsTraceDNSLookupInfo sInfo is being initialized with -1 instead of one of the valid nsTraceDNSLookupStatus enums. I suggest adding an additional enum value for -1 or some such.
Location - mozilla-trace.d, mozilla-trace.h.in: 
Where should we put them, mozilla/probes directory or mozilla/modules/probes?

"mozilla-trace.h.in is a generated file, why are we checking in
a copy of it?"
As you said it was so we could unconditionally include it later.If you want us not to not check it in, we can wrap the includes in the .cpp files. Just let us know. 

I'm traveling, but will roll a patch with the various changes early next week.
The attachment assumes the following probes are defined:

    probe xpc__wjs__entry(char *, char *, char *);
    probe xpc__wjs__return(char *, char *, char *);

I stuck the macro definitions in mozilla-load-trace.h.in, but that doesn't seem ideal.
The location doesn't really matter to me, someone should just make a judgement call and go with it.

Also, I didn't notice that you were including mozilla-trace.h directly into source files.  I still feel like you shouldn't be checking in a generated file that's going to be overwritten every time you use it.  I'd prefer you ifdef the includes.
Comment on attachment 283494 [details] [diff] [review]
XCPWrappedJSClass dtrace probes

>+    fp = JS_FrameIterator(cx, &iterator);
>+    fp = JS_FrameIterator(cx, &iterator);

That's a typo? (I don't know this code well enough to use a '.' instead of a '?')
You guys need to remember this rule, before any others: always ask me before making a new top-level directory. Oh, and rule #2: don't add more stuff under mozilla/modules. There, two rules and you're done. :-P

What would go in mozilla/probes? How many files, of what kinds? Examples and likely population and dependencies on other subdirs would help. Sorry if I'm missing this info in existing comments -- just point me to them if present.

/be
No problems - didn't know about the rules :)

mozilla/probes/
  mozilla-trace.d
  mozilla-trace-load.h.in 
[Contains support func extern "C" declarations and macros for the custom probes, appended to the generated mozilla-trace.h - was being used for load probes but is being used by others now, so should change this to mozilla-trace-probes.h.in]

  mozilla-trace.h.in 
[Generated by build, from the mozilla-trace.d file and used with the mozilla-trace-probes.h.in to create the final mozilla-trace.h file.]

  mozilla-trace.h 
[Generated by build by appending mozilla-trace.h.in and mozilla-trace-load.h.in Included by any sub modules with probes, link to it placed in dist/include]

If you check the following patch you can see examples of mozilla-trace.d, mozilla-trace-load.h.in, mozilla-trace.h.in:
https://bugzilla.mozilla.org/attachment.cgi?id=282438: 

Note we had checked in the generated mozilla-trace.h.in file to avoid wrapping mozilla-trace.h includes in the sub modules when on systems without dtrace, but as per Ted's request above we'll change this, wrapping the includes and no longer check this in.

So what we want to end up with after a build:
mozilla/probes/
  mozilla-trace.d
  mozilla-trace-probes.h.in 
  mozilla-trace.h [Generated by build, linked to in dist/include]
Robert: "xpc__wjs__entry( .... I stuck the macro definitions in mozilla-load-trace.h.in, but that doesn't seem ideal"

The idea behind putting all the support wrapper funcs, enums and structs for the probes into one top level header is that it will make it a lot easier for folks who want to use the probes to find the various support structs they need in their D Scripts. It's also handy for people who want to add extra probes to just put their bits into this one file and have plenty of examples in it to follow.

So in the next update to the patch I'd planned to change mozilla-trace-load.h.in to a more generic name mozilla-trace-probes.h.in

If you'd prefer to create a separate header.in for each probe set we can do this as well. Just let us know what you want to do.
This is an update based on the recent discussion in this bug. This change includes the framework only, no mentioning of any probes at all in this patch, those can come in later once the framework is in place IMO.

This creates a new top-level probes directory, with mozilla-trace.d in it. This patch includes zero generated files, all files are generated at *build time*, not a configure time. The generated file ends up in dist/include. Other than that this is the same change as the earlier patches (with maybe some minor tweaks here n' there, interdiff available if anyone cares).

John, if you can glance at this it'd be great too. Once this is in we can work on getting your load probes etc into the codebase as well.
Attachment #283749 - Flags: review?(ted.mielczarek)
Brendan, you cool with a new top-level probes directory here? For now it's got dtrace stuff only in it (i.e. Solaris only, and Leopard once available), and if dtrace is not enabled (default) the build system does noting in that directory. I can't really think of a better place for this, unless we want to stick this stuff in mozilla/build or something.
I would be ok with build if build guys are, since the current population is a bit thin to justify a new top-level, and the files are build-y. OTOH probes is good if we believe we'll add more files as we go (and perhaps we do so believe -- please comment).

/be
Comment on attachment 283749 [details] [diff] [review]
Mozilla Dynamic Tracing Framework (only), v11

>--- a/config/rules.mk
>+++ b/config/rules.mk
>+ifdef HAVE_DTRACE
>+ifdef DTRACE_PROBE_OBJ
...
>+ifdef NEED_DTRACE_PROBE_OBJ

Do we really need both of these?  I know you just carried this forward from the other patch, but this is confusing.  If we do need them both, then maybe they could get different names?  If not, let's just stick with one named DTRACE_PROBE_OBJ.  I'd still like to see if it could be auto-named from MODULE or something.

>--- a/configure.in
>+++ b/configure.in
>+  if test -n "$HAVE_DTRACE"; then
>+      AC_DEFINE(INCLUDE_MOZILLA_DTRACE)
>+  fi

This should have an else with an AC_MSG_ERROR.

r=me with those two issues addressed.
Attachment #283749 - Flags: review?(ted.mielczarek) → review+
Vlad, Stan: Do you guys have any thoughts on mozilla/probes vs mozilla/build? Vlad, are there files part of your non-dtrace probe stuff that could live with the dtrace probes etc? If so, would you like a separated dir over stuffing things into mozilla/build?
(In reply to comment #75)
> (From update of attachment 283749 [details] [diff] [review])
> >--- a/config/rules.mk
> >+++ b/config/rules.mk
> >+ifdef HAVE_DTRACE
> >+ifdef DTRACE_PROBE_OBJ
> ...
> >+ifdef NEED_DTRACE_PROBE_OBJ
> 
> Do we really need both of these?  I know you just carried this forward from the
> other patch, but this is confusing.  If we do need them both, then maybe they
> could get different names?  If not, let's just stick with one named
> DTRACE_PROBE_OBJ.  I'd still like to see if it could be auto-named from MODULE
> or something.

Padraig, John, any thoughts on this? It's not clear to me what's going on here.

> >--- a/configure.in
> >+++ b/configure.in
> >+  if test -n "$HAVE_DTRACE"; then
> >+      AC_DEFINE(INCLUDE_MOZILLA_DTRACE)
> >+  fi
> 
> This should have an else with an AC_MSG_ERROR.

Added, new patch coming up (with this issue fixed, and another quotation problem addressed too that I didn't catch earlier).

> r=me with those two issues addressed.

Thanks!
Johnny - talked with Padraig and it looks like the DTRACE_PROBE_OBJ is a left over that should have been removed. Will rip it out and test the latest patch, to make sure this is OK and post an update.
Johnny - took your patch and applied it by hand after removing the DTRACE_PROBE_OBJ sections. Seems to build fine.

I wanted to create an updated cvs patch, but I've just read only access to the repository and cvsdo will only let me add files, not directories, so I can't diff the contents of the probes dir. I know I could create a git repository and work around it, but if you could create a mozilla/probes dir that would be better.

I'll rework the load probes patch now to get it to build against this new framework.
(In reply to comment #80)
> Johnny - took your patch and applied it by hand after removing the
> DTRACE_PROBE_OBJ sections. Seems to build fine.
> 
> I wanted to create an updated cvs patch, but I've just read only access to the
> repository and cvsdo will only let me add files, not directories, so I can't
> diff the contents of the probes dir.

You might try upgrading cvsutils.  According to the changelog <http://www.red-bean.com/cvsutils/NEWS>, cvsdo supports adding directories in version 0.2.1 and up, and I was able to do so in 0.2.3 (on Linux, but it should work the same on Solaris):

myk@myk:~/cvsdotest$ cvs -d :pserver:anonymous@cvs-mirror.mozilla.org:/cvsroot co mozilla/client.mk
U mozilla/client.mk
myk@myk:~/cvsdotest$ cd mozilla
myk@myk:~/cvsdotest/mozilla$ mkdir foo
myk@myk:~/cvsdotest/mozilla$ touch foo/bar
myk@myk:~/cvsdotest/mozilla$ cvsdo add foo
myk@myk:~/cvsdotest/mozilla$ cvsdo add foo/bar
Use of uninitialized value in concatenation (.) or string at /usr/bin/cvsdo line 191, <ENTRIES> line 1.
myk@myk:~/cvsdotest/mozilla$ cvs diff -uN foo
cvs diff: Diffing foo
Index: foo/bar
===================================================================
RCS file: foo/bar
diff -N foo/bar
Thanks Myk - upgrade to 0.2.3 and it works a treat :)
New frame work patch incorporating Ted and Johnny's changes. Removed DTRACE_PROBE_OBJ as it was no longer required.

This patch will break the load probes. A new patch will be generated for them using the new framework structure. Will need to add additional code to probes/Makefile.in to concatenate the required headers and to the sub Makefiles so the mozilla-trace.d can be found under the probes dir.
Attachment #283749 - Attachment is obsolete: true
Attachment #283763 - Attachment is obsolete: true
Comment on attachment 284031 [details] [diff] [review]
Mozilla Dynamic Tracing Framework (only), v12

Asking sayrer for second-review
Attachment #284031 - Flags: review?(sayrer)
Comment on attachment 284031 [details] [diff] [review]
Mozilla Dynamic Tracing Framework (only), v12

looks good to me
Attachment #284031 - Flags: review?(sayrer) → review+
Attachment #284031 - Flags: approval1.9?
Comment on attachment 284031 [details] [diff] [review]
Mozilla Dynamic Tracing Framework (only), v12

a=me, let's get this landed!
Attachment #284031 - Flags: approval1.9? → approval1.9+
I landed the dtrace framework patch today (with some slight whitespace tweaks). That turned the world red, due to mozilla/probes not being part of the list of directories that's checked out by client.mk, so I fixed that as well.

I don't know if we want to close this bug now, and move the layout (etc) probe discussion to a new bug, or keep this bug open for that work. I don't really care either way...
This bug is more than long enough. Let's file additional bugs for more work... I'd in particular like cycle collection probes.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Agree that another bug would be a good idea.

Do we want separate ones for each probe set? The difficulty with this is that they are all included in the mozilla-trace.d file as they are all in the mozilla provider.

So for consistency it probably makes sense to collect them into one patch and land groups of probes as we go. Each time after we land a set starting up a new bug to catch the next group. This way we should be able to ensure consistency across the probes with regard to layout, implementation and so on.

I have a patch for the load and dnslookup probes building and running against the new framework, so just want to know which bug to put this patch against. I'd like to see Robert's XPC_WJS probes in as well and some layout probes, based on the early patch but modified as per Dave B recomendations.

What do folks think?
Mozilla load and dns lookup probes patch, changed to build and run with the new framework. Assume this will be moved to another Probes bug, but wanted to get them up for others to take a look.
We can fold in Robert's probes to this and add new ones for Benjamin's cycle collections probes. Or we can split them up, up to you guys.

Supports:
 load__init (void * unique_id, nsTraceLoadType type, struct nsTraceLoadInfo *info)
 load__start (void * unique_id, nsTraceLoadType type, struct nsTraceLoadInfo *info)
 load__done  (void * unique_id, nsTraceLoadType type, struct nsTraceLoadInfo *info)
 dnslookup__init (void * unique_id, struct nsTraceDNSLookupInfo *info)
 dnslookup__start (void * unique_id, struct nsTraceDNSLookupInfo *info)
 dnslookup__done  (void * unique_id, struct nsTraceDNSLookupInfo *info)
Attachment #282440 - Attachment is obsolete: true
Blocks: 410588
I just created Bug 410588 to track adding the load/lookup probes, John could you attach your latest patch there please?

Blocks: 401697
You need to log in before you can comment on or make changes to this bug.