Closed Bug 698201 Opened 13 years ago Closed 13 years ago

Remove tracer

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: evilpie, Assigned: dvander)

References

Details

Attachments

(8 files)

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?
Blocks: 647628
Depends on: 693815
Wait, I thought this wasn't happening until 11?
This is just for bookkeeping. It is supposed to be done after FF10 merges to Aurora.
Note: don't forget the tracer-related stuff in xpconnect/src/XPCJSRuntime.cpp.
Target Milestone: --- → mozilla11
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)
Attachment #573314 - Flags: review? → review?(bhackett1024)
Attachment #573317 - Flags: superreview?(mrbkap)
Attachment #573317 - Flags: review?(mrbkap)
Comment on attachment 573311 [details] [diff] [review]
part 2: remove imacros

r++++++ would review again
Attachment #573311 - Flags: review?(luke) → review+
Attachment #573313 - Flags: review?(luke) → review+
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 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+
Can you run "diffstats" on the combined patches? :)
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 on attachment 573316 [details] [diff] [review]
part 6: xpconnect changes

\o/
Attachment #573316 - Flags: review?(bobbyholley+bmo) → review+
(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(-)
Attachment #573310 - Flags: review?(dmandelin) → review+
Ship it, I say!
Assignee: general → dvander
Status: NEW → ASSIGNED
Whiteboard: [MemShrink]
... Once it passes try.

https://tbpl.mozilla.org/?tree=Try&rev=7e79a6ec0b0c

doesn't look good.
Whiteboard: [MemShrink] → [MemShrink:P1]
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]
And by "turned off" I mean "not built by default".
Blocks: 692838
\o/

Why is "jstracer.cpp" still there?
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.
Thanks, I'm not sure how that happened but I pushed a follow-up fix:

http://hg.mozilla.org/integration/mozilla-inbound/rev/4864134dad4e
Attachment #576516 - Flags: review?(dvander)
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+
Follow-up to remove jstracer.cpp merged to trunk:
https://hg.mozilla.org/mozilla-central/rev/4864134dad4e
Whiteboard: [inbound]
Blocks: 705168
There are still the javascript.options.jitprofiling about:config voices. Shouldn't they be removed?
(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?)
Depends on: 705380
My patch in bug 705356 removes the profiling options as well. Let's use that bug for that discussion.
Depends on: 706611
Depends on: 707341
Depends on: 707351
Depends on: 709792
Depends on: 710481
Depends on: 710486
Depends on: 714894
Depends on: 751280
You need to log in before you can comment on or make changes to this bug.