Closed
Bug 698201
Opened 13 years ago
Closed 13 years ago
Remove tracer
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: evilpie, Assigned: dvander)
References
Details
Attachments
(8 files)
2.27 MB,
patch
|
dmandelin
:
review+
|
Details | Diff | Splinter Review |
38.10 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
97.57 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
55.43 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
88.27 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
42.80 KB,
patch
|
bholley
:
review+
|
Details | Diff | Splinter Review |
7.52 KB,
patch
|
gal
:
review+
gal
:
superreview+
|
Details | Diff | Splinter Review |
20.96 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
The time has come. tom@tom-linux:~/inbound/js/src$ hg log jstracer.cpp | grep -i "bug" -c 1474 Brain thinks dvander has a patch for this?
Reporter | ||
Updated•13 years ago
|
Comment 1•13 years ago
|
||
Wait, I thought this wasn't happening until 11?
Reporter | ||
Comment 2•13 years ago
|
||
This is just for bookkeeping. It is supposed to be done after FF10 merges to Aurora.
Comment 3•13 years ago
|
||
Note: don't forget the tracer-related stuff in xpconnect/src/XPCJSRuntime.cpp.
Updated•13 years ago
|
Target Milestone: --- → mozilla11
Assignee | ||
Comment 4•13 years ago
|
||
Remove nanojit/*, jstracer*, lirasm/*, and friends. This queue is based on top of Bill's write barriers patch. I'll send it to try soon - the browser builds locally and tests pass.
Attachment #573310 -
Flags: review?(dmandelin)
Assignee | ||
Comment 5•13 years ago
|
||
Attachment #573311 -
Flags: review?(luke)
Assignee | ||
Comment 6•13 years ago
|
||
Attachment #573313 -
Flags: review?(luke)
Assignee | ||
Comment 7•13 years ago
|
||
Attachment #573314 -
Flags: review?
Assignee | ||
Updated•13 years ago
|
Attachment #573314 -
Flags: review? → review?(bhackett1024)
Assignee | ||
Comment 8•13 years ago
|
||
Attachment #573315 -
Flags: review?(luke)
Assignee | ||
Comment 9•13 years ago
|
||
Attachment #573316 -
Flags: review?(bobbyholley+bmo)
Assignee | ||
Comment 10•13 years ago
|
||
Attachment #573317 -
Flags: superreview?(mrbkap)
Attachment #573317 -
Flags: review?(mrbkap)
Comment 11•13 years ago
|
||
Comment on attachment 573311 [details] [diff] [review] part 2: remove imacros r++++++ would review again
Attachment #573311 -
Flags: review?(luke) → review+
Updated•13 years ago
|
Attachment #573313 -
Flags: review?(luke) → review+
Comment 12•13 years ago
|
||
Comment on attachment 573315 [details] [diff] [review] part 5: remove structures, functions I'd be inclined to just remove JSOPTION_JIT.
Attachment #573315 -
Flags: review?(luke) → review+
Comment 13•13 years ago
|
||
Comment on attachment 573314 [details] [diff] [review] part 4: remove everything def'd under JS_TRACER Review of attachment 573314 [details] [diff] [review]: ----------------------------------------------------------------- What is the plan for the deprecated API bits like JSOPTION_JIT and JS_SetGCParameterForThread (and no-ops below it)?
Attachment #573314 -
Flags: review?(bhackett1024) → review+
Comment 14•13 years ago
|
||
Can you run "diffstats" on the combined patches? :)
Comment 15•13 years ago
|
||
Comment on attachment 573317 [details] [diff] [review] part 7: remove browser prefs r=me, the tracer shall rise again, if it makes sense down the road
Attachment #573317 -
Flags: superreview?(mrbkap)
Attachment #573317 -
Flags: superreview+
Attachment #573317 -
Flags: review?(mrbkap)
Attachment #573317 -
Flags: review+
Comment 16•13 years ago
|
||
Comment on attachment 573316 [details] [diff] [review] part 6: xpconnect changes \o/
Attachment #573316 -
Flags: review?(bobbyholley+bmo) → review+
Assignee | ||
Comment 17•13 years ago
|
||
(In reply to Brian Hackett from comment #13) > What is the plan for the deprecated API bits like JSOPTION_JIT and > JS_SetGCParameterForThread (and no-ops below it)? I'll get rid of JSOPTION_JIT. Do you think I should get rid of the other ones as well? It doesn't seem like we have plans for them. (In reply to Nicholas Nethercote [:njn] from comment #14) > Can you run "diffstats" on the combined patches? :) 350 files changed, 126 insertions(+), 67643 deletions(-)
Updated•13 years ago
|
Attachment #573310 -
Flags: review?(dmandelin) → review+
Updated•13 years ago
|
Whiteboard: [MemShrink]
Comment 19•13 years ago
|
||
... Once it passes try. https://tbpl.mozilla.org/?tree=Try&rev=7e79a6ec0b0c doesn't look good.
Updated•13 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P1]
Comment 20•13 years ago
|
||
Actually, since the tracer was already turned off, removing it from the codebase won't make any difference to memory usage. So I've removed the MemShrink tag.
Whiteboard: [MemShrink:P1]
Comment 21•13 years ago
|
||
And by "turned off" I mean "not built by default".
Assignee | ||
Comment 22•13 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/b2e8d10f25a1 http://hg.mozilla.org/integration/mozilla-inbound/rev/565abbb3f5d7 http://hg.mozilla.org/integration/mozilla-inbound/rev/1d20af78b178 http://hg.mozilla.org/integration/mozilla-inbound/rev/b1c5b23aa0fa http://hg.mozilla.org/integration/mozilla-inbound/rev/b02f12cc9656 http://hg.mozilla.org/integration/mozilla-inbound/rev/37cbe2c24ecd http://hg.mozilla.org/integration/mozilla-inbound/rev/c28fbe8374b4
Whiteboard: [inbound]
Comment 23•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/b2e8d10f25a1 http://hg.mozilla.org/mozilla-central/rev/565abbb3f5d7 http://hg.mozilla.org/mozilla-central/rev/1d20af78b178 http://hg.mozilla.org/mozilla-central/rev/b1c5b23aa0fa http://hg.mozilla.org/mozilla-central/rev/b02f12cc9656 http://hg.mozilla.org/mozilla-central/rev/37cbe2c24ecd http://hg.mozilla.org/mozilla-central/rev/c28fbe8374b4 \o/
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 24•13 years ago
|
||
\o/ Why is "jstracer.cpp" still there?
Comment 25•13 years ago
|
||
jandem pointed out in IRC that the removal of jstracer.cpp is present in the reviewed patch attached to the bug, but not in the patch landed on inbound.
Assignee | ||
Comment 26•13 years ago
|
||
Thanks, I'm not sure how that happened but I pushed a follow-up fix: http://hg.mozilla.org/integration/mozilla-inbound/rev/4864134dad4e
Reporter | ||
Comment 27•13 years ago
|
||
Attachment #576516 -
Flags: review?(dvander)
Assignee | ||
Comment 28•13 years ago
|
||
Comment on attachment 576516 [details] [diff] [review] Remove some left-overs from JM Review of attachment 576516 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, but let's do follow-up bugs for future tracer-ectomies. ::: js/src/methodjit/MethodJIT-inl.h @@ +90,5 @@ > JITScriptStatus status = script->getJITStatus(fp->isConstructing()); > if (status == JITScript_Invalid) > return Compile_Abort; > if (status == JITScript_None && !cx->hasRunOption(JSOPTION_METHODJIT_ALWAYS)) { > + if ((cx->typeInferenceEnabled()) Nit: extra parens
Attachment #576516 -
Flags: review?(dvander) → review+
Comment 29•13 years ago
|
||
Follow-up to remove jstracer.cpp merged to trunk: https://hg.mozilla.org/mozilla-central/rev/4864134dad4e
Whiteboard: [inbound]
Comment 30•13 years ago
|
||
There are still the javascript.options.jitprofiling about:config voices. Shouldn't they be removed?
Comment 31•13 years ago
|
||
(In reply to Zio_Fede from comment #30) > There are still the javascript.options.jitprofiling about:config voices. > Shouldn't they be removed? Followup, please. (But doesn't that apply to other jits as well?)
Comment 32•13 years ago
|
||
My patch in bug 705356 removes the profiling options as well. Let's use that bug for that discussion.
You need to log in
before you can comment on or make changes to this bug.
Description
•