625962 - unbitrot shark profiling

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andreas Gal :gal]

I also renamed all the functions to startProfiling/stopProfiling. All other profiling types should be folded under this. Its pointless to pollute the browser with so much special case code (JProf/vtune/callgrind). We might even want to give startProfiling() an extra arg to select which profile to start in case a platform has more than one. I also added JS_DefineProfilingFunctions() to common out the code in the 6 different places we currently define these functions.

Comment 1

[Andreas Gal :gal]

Attachment: patch

Comment 2

[Andreas Gal :gal]

Attachment: patch

Comment 3

[Andreas Gal :gal]

Attachment: patch

Comment 4

[Robert Sayre]

Comment on attachment 504037 [details] [diff] [review]
patch

I think people using 10.5 for Shark might not like us removing this code if sharkctl.cpp doesn't work there. We can always put it back.

Comment 5

[Andreas Gal :gal]

Attachment: patch

Comment 6

[Andreas Gal :gal]

Attachment: patch

Comment 7

[Andreas Gal :gal]

Comment on attachment 504039 [details] [diff] [review]
patch

Carrying sayrer's review forward.

Comment 8

[Patrick Walton (:pcwalton)]

Comment on attachment 504037 [details] [diff] [review]
patch

Looks good to me, except for sharkctl.cpp missing.

Comment 9

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/699a16e77464

Comment 10

[Boris Zbarsky [:bzbarsky]]

So this basically fixed bug 595748, right?

Comment 12

[Boris Zbarsky [:bzbarsky]]

I filed bug 625993 on restoring the old shark functions so dromaeo's &shark parameter, mats' instrumented peacekeeper, and other things like that will keep working.

Comment 13

[Boris Zbarsky [:bzbarsky]]

So I tried using this today.  It has a slight issue: see bug 626437

Comment 14

[Chris Leary [:cdleary] (not checking bugmail)]

cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/699a16e77464
http://hg.mozilla.org/mozilla-central/rev/c94d2a07e3f5

Comment 15

[Mike Hommey [:glandium]]

Why was the following change, which appears in the commit but not in the reviewed patch, needed ?

diff --git a/config/rules.mk b/config/rules.mk
--- a/config/rules.mk
+++ b/config/rules.mk
@@ -219,17 +219,17 @@ endif # XPCSHELL_TESTS
 ifdef CPP_UNIT_TESTS
 
 # Compile the tests to $(DIST)/bin.  Make lots of niceties available by default
 # through TestHarness.h, by modifying the list of includes and the libs against
 # which stuff links.
 CPPSRCS += $(CPP_UNIT_TESTS)
 SIMPLE_PROGRAMS += $(CPP_UNIT_TESTS:.cpp=$(BIN_SUFFIX))
 INCLUDES += -I$(DIST)/include/testing
-LIBS += $(XPCOM_GLUE_LDOPTS) $(NSPR_LIBS)
+LIBS += $(XPCOM_GLUE_LDOPTS) $(NSPR_LIBS) -ljs_static
 
 # ...and run them the usual way
 check::
        @$(EXIT_ON_ERROR) \
          for f in $(subst .cpp,$(BIN_SUFFIX),$(CPP_UNIT_TESTS)); do \
            XPCOM_DEBUG_BREAK=stack-and-abort $(RUN_TEST_PROGRAM) $(DIST)/bin/$$f; \
          done