Closed Bug 625962 Opened 9 years ago Closed 9 years ago

unbitrot shark profiling

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: gal, Assigned: gal)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file, 4 obsolete files)

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.
Attached patch patch (obsolete) — Splinter Review
Assignee: general → gal
Attached patch patch (obsolete) — Splinter Review
Attachment #504031 - Attachment is obsolete: true
Attached patch patch (obsolete) — Splinter Review
Attachment #504035 - Attachment is obsolete: true
Attachment #504037 - Flags: review?(sayrer)
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.
Attachment #504037 - Flags: review?(sayrer) → review+
Attached patch patch (obsolete) — Splinter Review
Attachment #504037 - Attachment is obsolete: true
Attachment #504038 - Flags: review?(sayrer)
Attached patch patchSplinter Review
Attachment #504038 - Attachment is obsolete: true
Attachment #504039 - Flags: review?(sayrer)
Attachment #504038 - Flags: review?(sayrer)
Comment on attachment 504039 [details] [diff] [review]
patch

Carrying sayrer's review forward.
Attachment #504039 - Flags: review?(sayrer) → review+
Comment on attachment 504037 [details] [diff] [review]
patch

Looks good to me, except for sharkctl.cpp missing.
Attachment #504037 - Flags: feedback+
http://hg.mozilla.org/tracemonkey/rev/699a16e77464
Whiteboard: fixed-in-tracemonkey
So this basically fixed bug 595748, right?
Duplicate of this bug: 595748
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.
Depends on: 625993
So I tried using this today.  It has a slight issue: see bug 626437
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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
You need to log in before you can comment on or make changes to this bug.