The default bug view has changed. See this FAQ.

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: evilpie, Assigned: dvander)

Tracking

unspecified
mozilla11
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(8 attachments)

(Reporter)

Description

6 years ago
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

6 years ago
Blocks: 647628
Depends on: 693815
Wait, I thought this wasn't happening until 11?
(Reporter)

Comment 2

6 years ago
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
(Assignee)

Comment 4

5 years ago
Created attachment 573310 [details] [diff] [review]
part 1: remove major files

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

5 years ago
Created attachment 573311 [details] [diff] [review]
part 2: remove imacros
Attachment #573311 - Flags: review?(luke)
(Assignee)

Comment 6

5 years ago
Created attachment 573313 [details] [diff] [review]
part 3: remove traceable natives from js/src
Attachment #573313 - Flags: review?(luke)
(Assignee)

Comment 7

5 years ago
Created attachment 573314 [details] [diff] [review]
part 4: remove everything def'd under JS_TRACER
Attachment #573314 - Flags: review?
(Assignee)

Updated

5 years ago
Attachment #573314 - Flags: review? → review?(bhackett1024)
(Assignee)

Comment 8

5 years ago
Created attachment 573315 [details] [diff] [review]
part 5: remove structures, functions
Attachment #573315 - Flags: review?(luke)
(Assignee)

Comment 9

5 years ago
Created attachment 573316 [details] [diff] [review]
part 6: xpconnect changes
Attachment #573316 - Flags: review?(bobbyholley+bmo)
(Assignee)

Comment 10

5 years ago
Created attachment 573317 [details] [diff] [review]
part 7: remove browser prefs
Attachment #573317 - Flags: superreview?(mrbkap)
Attachment #573317 - Flags: review?(mrbkap)

Comment 11

5 years ago
Comment on attachment 573311 [details] [diff] [review]
part 2: remove imacros

r++++++ would review again
Attachment #573311 - Flags: review?(luke) → review+

Updated

5 years ago
Attachment #573313 - Flags: review?(luke) → review+

Comment 12

5 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 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 15

5 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 on attachment 573316 [details] [diff] [review]
part 6: xpconnect changes

\o/
Attachment #573316 - Flags: review?(bobbyholley+bmo) → review+
(Assignee)

Comment 17

5 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(-)
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".
(Reporter)

Updated

5 years ago
Blocks: 692838
Blocks: 704356
(Assignee)

Comment 22

5 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]
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Comment 24

5 years ago
\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.
(Assignee)

Comment 26

5 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

5 years ago
Created attachment 576516 [details] [diff] [review]
Remove some left-overs from JM
Attachment #576516 - Flags: review?(dvander)
(Assignee)

Comment 28

5 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+
Follow-up to remove jstracer.cpp merged to trunk:
https://hg.mozilla.org/mozilla-central/rev/4864134dad4e
Whiteboard: [inbound]
(Reporter)

Updated

5 years ago
Blocks: 705168

Comment 30

5 years ago
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?)
Blocks: 705356
Depends on: 705380
My patch in bug 705356 removes the profiling options as well. Let's use that bug for that discussion.
Blocks: 705401
Depends on: 706611
Depends on: 707341
Depends on: 707351

Updated

5 years ago
Depends on: 709792
Depends on: 710481
Depends on: 710486

Updated

5 years ago
Blocks: 714052
(Reporter)

Updated

5 years ago
Depends on: 714894

Updated

5 years ago
Depends on: 751280
You need to log in before you can comment on or make changes to this bug.