449452 - [jsd] Modernize jsdIDebuggerService: jsd2

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[John J. Barton]

This is a collector bug for a set of proposed changes to our friend jsd.

Comment 1

[John J. Barton]

I've put bug 379410 here meaning "I wish we had a jsd API like the one Igor proposes for C++".  The basic idea is to call onScriptsCreated once per compilation unit rather than calling onScriptCreated once for every script in a compilation unit. 

Comment 2

[:Gijs (out for now; he/him)]

I'm wondering if we should spend time working on the implementation of this in SpiderMonkey rather than Tamarin. (Please bear in mind that I've been away for 2 months and have no idea about the decisions/releases that have been made/planned in that timeframe, and the current state of debugging in Tamarin)

Could someone shed some light on what would make sense in this respect?

Comment 3

[Mike Shaver (:shaver emeritus)]

It makes sense to improve the SpiderMonkey API; we will be on SpiderMonkey for the foreseeable future.

Comment 4

[John J. Barton]

Bug 462704 and bug 463239 are two errors in jsd, one prevents debugging, one causes incorrect execution when you debug. They may affect all js debugging, jsd or not.

The firefox extension:
http://fbug.googlecode.com/svn/chromebug/tests/[e-mail]

starts jsd and runs the test cases for these bugs. (Tools -> Test Now or run firefox from the command line with -chrome chrome://jsdtest/content/testWebPage.html)

Comment 5

[:Gijs (out for now; he/him)]

Any particular reason those bugs were in Firefox>General, and one is filed against trunk, and one against 3.0 Branch? Did you test both in 3.0, both in Trunk, or is it really as the bugs dictate?

Comment 6

[John J. Barton]

Is there a better category than General? Tests were all against FF 3.0.3

Comment 7

[:Gijs (out for now; he/him)]

(In reply to comment #6)
> Is there a better category than General? Tests were all against FF 3.0.3

First of all, then the bugs should reflect that, and it would be good to assess if the same failures occur on trunk.

As for categories, you claimed both of these problems are JSD failures, which I presume you've somehow verified, in which case they should be under Other Apps --> JavaScript Debugger, as that is what's in use for JSD backend bugs at the moment.

Comment 8

[John J. Barton]

(In reply to comment #7)
> As for categories, you claimed both of these problems are JSD failures, which I
> presume you've somehow verified, in which case they should be under Other Apps
> --> JavaScript Debugger, as that is what's in use for JSD backend bugs at the
> moment.

Maybe I'm using the wrong term. The bugs were tested against jsdIDebuggerService. 

I entered the bugs via https://bugzilla.mozilla.org/enter_bug.cgi.  There is no JavaScript Debugger category on that form. I can see it on this page now so I'll look for it next time.

Comment 9

[John J. Barton]

I see that these 'jsd' bug reports have been set to [e-mail]. I don't know what that means; none of these can be fixed in venkman and all require C++ code changes.

Comment 10

[Rob Campbell [:rc] (:robcee)]

(In reply to comment #9)
> I see that these 'jsd' bug reports have been set to [e-mail]. I
> don't know what that means; none of these can be fixed in venkman and all
> require C++ code changes.

that's an alias so we can track jsd bugs by following them in bugzilla's email prefs. Something more appropriate might be better though. e.g., [e-mail]?

Comment 11

[:Gijs (out for now; he/him)]

(In reply to comment #10)
> (In reply to comment #9)
> > I see that these 'jsd' bug reports have been set to [e-mail]. I
> > don't know what that means; none of these can be fixed in venkman and all
> > require C++ code changes.
> 
> that's an alias so we can track jsd bugs by following them in bugzilla's email
> prefs. Something more appropriate might be better though. e.g.,
> [e-mail]?

So it would be if this component was only for JSD. As it is, it isn't, so changing the alias would simply make *another* set of bugs have a "less appropriate" alias.

We may want to consider getting a separate bugzilla component for JSD, but another bug would have to be filed for that.

Comment 12

[John J. Barton]

The setting does not bother me as long as we know what it means.

Comment 13

[John J. Barton]

BTW the [jsd] in the titles of bugs is very helpful to me since I have deal with bugs from may other areas.

Comment 14

[John J. Barton]

Here are some observations by boris concerning jsd overhead. (>my replies)

The synposis: jsd.on() cause 10% overhead 100% of the time js is running; Firebug calls jsd.on() in common use cases.

--------
I was just profiling a testcase in a build that happened to have Firebug installed (but not enabled on that page!), and noticed that 10% of the testcase execution time was under jsd_ObjectHook.  It looks like it does locking-and-stuff on every single object allocation.

I looked around, and the hook is installed if someone calls jsdService::On.

Is Firebug calling this during startup or something?  If so, why? 
-------
>
> No, in fact we try to turn object trace off. Of course that does not mean we succeed.
...
>            jsd.on();

OK, so you _are_ calling it during startup.  Is it possible to stop doing that?

>            jsd.flags |= DISABLE_OBJECT_TRACE;

All that does is keep us from walking the stack on every allocation looking for the script and line number that caused it.  We still track every single object allocation, create a new object representing it, mess with hashtables, lock things, etc, etc.

See http://mxr.mozilla.org/mozilla-central/source/js/jsd/jsd_obj.c#114 and
http://mxr.mozilla.org/mozilla-central/source/js/jsd/jsd_obj.c#160

I have no idea whether all that work jsd does there is really needed if the DISABLE_OBJECT_TRACE flag is set, and in particular whether other parts of jsd depend on jsdc->objectsTable.  Looks like at least jsdValue::GetObjectValue does.
--------

> and not disabled with pause() or the flag setting?

The flag setting just makes the hook less expensive; the time it takes would presumably be even more than the 10% I saw if you didn't set the flag.

jsdService::On sets up all sorts of hooks that pause() does not remove.  Specifically, everything that is set up by jsd_DebuggerOnForUser:

207     JS_SetNewScriptHookProc(jsdc->jsrt, jsd_NewScriptHookProc, jsdc);
208     JS_SetDestroyScriptHookProc(jsdc->jsrt, jsd_DestroyScriptHookProc, jsdc);
209     JS_SetDebuggerHandler(jsdc->jsrt, jsd_DebuggerHandler, jsdc);
210     JS_SetExecuteHook(jsdc->jsrt, jsd_TopLevelCallHook, jsdc);
211     JS_SetCallHook(jsdc->jsrt, jsd_FunctionCallHook, jsdc);
212     JS_SetObjectHook(jsdc->jsrt, jsd_ObjectHook, jsdc);
213     JS_SetThrowHook(jsdc->jsrt, jsd_ThrowHandler, jsdc);
214     JS_SetDebugErrorHook(jsdc->jsrt, jsd_DebugErrorHook, jsdc);
-----

> Does pause/unpause affect this work?

No.


> We could experiment with replacing pause/unpause with off/on, but I assumed that the whole reason pause/unpause existed was because on/off was expensive or otherwise undesirable.

I would assume that off/on means you lose information about any already-allocated objects, for example.  I have no idea whether that's ok in your case.
----
> But as far as I understand these 'hooks' are just C shells for JS hooks. 

They're not.  For example, here's jsd_ObjectHook:

160 jsd_ObjectHook(JSContext *cx, JSObject *obj, JSBool isNew, void *closure)
161 {
162     JSDObject* jsdobj;
163     JSDContext* jsdc = (JSDContext*) closure;
164
165     if( ! jsdc || ! jsdc->inited )
166         return;
167
168     JSD_LOCK_OBJECTS(jsdc);
169     if(isNew)
170     {
171         jsdobj = _createJSDObject(jsdc, cx, obj);
172         TRACEOBJ(jsdc, jsdobj, 0);
173     }
174     else
175     {
176         jsdobj = jsd_GetJSDObjectForJSObject(jsdc, obj);
177         if( jsdobj )
178         {
179             TRACEOBJ(jsdc, jsdobj, 1);
180             _destroyJSDObject(jsdc, jsdobj);
181         }
182     }
183     JSD_UNLOCK_OBJECTS(jsdc);
184 }

There's no js hook here; this is all just bookkeeping work for use elsewhere.

Similarly, jsd_NewScriptHookProc will create a new JSDScript from tehe given JSScript no matter what, even if there's no JS new script hook to call.  See http://mxr.mozilla.org/mozilla-central/source/js/jsd/jsd_scpt.c#570

> It sounds like no matter what JS value the ObjectHook had, the overhead would be in place?

There's no JS ObjectHook that I can see (which makes sense, since the jsd_ObjectHook is called at a point at which calling JS is unsafe).

Comment 15

[:Gijs (out for now; he/him)]

Right, there are bugs 483685,483681,483282 on related stuff.

I haven't looked at the specific case, but in general perf in JSD is difficult because its bookkeeping is necessary later, when the user wants to use their debugger and basically wants it to (5 minutes ago) have gotten itself a handle to the bytecode map for the script they then want to look at. In other words, the current APIs are such that bookkeeping beforehand is necessary to be able to debug later. I am not sure if it is possible to change that for the object case, I don't know enough about it.

Bug 480765 tried to address that (by being able to retrieve a jsdscript after the fact), but I'm still stuck with bug 483685 trying to actually reliably use that feature - that is, it doesn't seem to be working for me/Venkman, and I am not sure why.

Comment 16

[Rob Campbell [:rc] (:robcee)]

following up here rather than the email thread, is it possible to instrument calls to jsd.on() in a debug environment to try to nail down where this is happening in Firebug code? We've seen some nebulous "Firebug is slow" complaints on the web and I'd be curious if this is the cause.

Comment 17

[John J. Barton]

I am answering Gijs' question from bug 449464 here:
> If you've got proposals on how to reduce the perf hit Firefox currently takes
> from enabling JSD, please do feel free to file separate bugs for them.

The basic cause of jsd performance problems is simple: jsd operates at the XUL application level but developers work at the nsIDOMWindow (JS execution context) level. jsd does work for every context, not just the one the developer wants information from. Imagine Visual Studio debugging firefox by using operations on every application in the system, including the entire Visual Studio UI. Sounds dumb huh? Well that is jsd.

Unless we solve this, all the other fixes are just window dressing. (if you can pardon the pun).

Comment 18

[Boris Zbarsky [:bzbarsky]]

> The basic cause of jsd performance problems is simple

Maybe.  The basic causes of the performance hit that I've seen in my profiles are actually quite present even if I only have one tab and one window open.  Specifically:

1) jsd/firebug does a bunch of work on every single script compilation, even if
   the script won't be debugged, and even if it's inactive, as far as I can see.
   This noticeably affects pageload time.
2) Having the debugger actually active forces the JavaScript engine to
   deoptimize various things to keep its API promises to the debugger. 

I agree that not knowing what you plan to debug is a problem, but there are serious problems other than that...

Comment 19

[John J. Barton]

(In reply to comment #18)
> > The basic cause of jsd performance problems is simple
> 
> Maybe.  The basic causes of the performance hit that I've seen in my profiles
> are actually quite present even if I only have one tab and one window open. 
> Specifically:
> 
> 1) jsd/firebug does a bunch of work on every single script compilation, even if
>    the script won't be debugged, and even if it's inactive, as far as I can
> see.
>    This noticeably affects pageload time.

On the first script compile we two two very ugly things:
  A) inject the console, 
  B) implement the equivalent of bug 449464 by setting a breakpoint in the outerScript then running a bunch of JS.

You can get a feel for these by disabling the console (A) or script panel (B).

In Firebug 1.6 I hope 449464 will dramatically reduce B.
In Firebug 1.7 I hope we can re implement the console based on your XHR suggestion and/or Bug 529474  

Of course we could be doing stupid stuff in addition. We could try another performance profile to look for this.

> 2) Having the debugger actually active forces the JavaScript engine to
>    deoptimize various things to keep its API promises to the debugger. 

Yes, this is one increasingly important form of my point above: if the deoptimize was limited to the window we were debugging, the performance of Firefox would be high except on the the part we are studying with the debugger.

Comment 20

[Boris Zbarsky [:bzbarsky]]

(In reply to comment #19)
> On the first script compile we two two very ugly things:

The issue I was seeing seemed to happen on all compiles; it looked like the script text was being analyzed or something...  Maybe that behavior has been changed since?  It's been a few months.

> Yes, this is one increasingly important form of my point above: if the
> deoptimize was limited to the window we were debugging, the performance of
> Firefox would be high except on the the part we are studying with the debugger.

Possibly worth filing bugs to make the debugger hooks per-context instead of per-runtime?

Comment 21

[John J. Barton]

Currently jsd.onThrow() is called on the |throw| and on each |catch| block that fails.  However there is no signal that |catch| succeeded nor one that control flow passed out of the oldest frame (uncaught exception). If there is a chance someone would work on onCatch/onUncaughtException, I'll open a bug.