Bug 21345 - Start the debugger without reloading the inspected page
Summary: Start the debugger without reloading the inspected page
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
: 21616 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-10-03 13:12 PDT by Geoffrey Garen
Modified: 2008-10-15 19:36 PDT (History)
0 users

See Also:


Attachments
patch (35.99 KB, patch)
2008-10-03 13:17 PDT, Geoffrey Garen
no flags Details | Formatted Diff | Diff
patch (39.56 KB, patch)
2008-10-03 13:20 PDT, Geoffrey Garen
no flags Details | Formatted Diff | Diff
patch, merged to ToT, with a couple small changes (40.95 KB, patch)
2008-10-09 01:45 PDT, Darin Adler
no flags Details | Formatted Diff | Diff
patch, finished product with no known bugs (79.32 KB, patch)
2008-10-15 13:30 PDT, Geoffrey Garen
zwarich: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2008-10-03 13:12:26 PDT
Patch coming.
Comment 1 Geoffrey Garen 2008-10-03 13:17:38 PDT
Created attachment 24065 [details]
patch

This is a work-in-progress patch for dynamically recompiling with debugger / profiler hooks. Currently, it's set up to recompile on a timer, or when you attach the debugger. But there are problems:

* Seem to be some cases where a function doesn't round-trip correctly from source
* Recompiling on a timer seems to break event handling in dojo demos, gmail.com, and the debugger
* The debugger sometimes crashes

Also, the final version would need to add profiler hook opcodes to the machine, along with logic for emitting them when appropriate, in order to get the speedup.

I was hoping to have this done by the time I left for vacation, but it didn't come together, so I'm posting it in case anyone wants to have a crack at finishing it while I'm gone.
Comment 2 Geoffrey Garen 2008-10-03 13:20:36 PDT
Created attachment 24066 [details]
patch

Added a missing file to the patch.
Comment 3 Darin Adler 2008-10-09 00:43:28 PDT
I'm merging this patch with TOT so I can do some testing.
Comment 4 Darin Adler 2008-10-09 01:45:17 PDT
Created attachment 24228 [details]
patch, merged to ToT, with a couple small changes

I merged. I fixed a couple issues I noticed.

I changed the code that recompiles functions to use ProtectedPtr for the JSFunction objects. But maybe that's not needed, because nothing can be allocated during the recompile timer?

I changed the recompile timer to call JSGlobalObject::globalExec() instead of JSDOMWindowBase::globalExec(), because otherwise we'll die when recompiling functions attached to pages that are closed and have no frames.

I did a few other tweaks.

So far I haven't been able to get much of this to work. I die inside assertions in the debugger and I can't even tell if they're caused by the patch or not.
Comment 5 Darin Adler 2008-10-09 01:46:34 PDT
I think the Page.h/cpp part of this patch should probably be dumped.
Comment 6 Darin Adler 2008-10-09 01:47:15 PDT
Since nothing starts the timer! Maybe it just happened to be turned off when Geoff posted the patch?
Comment 7 Geoffrey Garen 2008-10-14 11:27:28 PDT
> Since nothing starts the timer! Maybe it just happened to be turned off when
> Geoff posted the patch?

Yeah, I posted the patch with the timer turned off. I was using the timer as a stress tester.
Comment 8 Geoffrey Garen 2008-10-15 13:30:06 PDT
Created attachment 24367 [details]
patch, finished product with no known bugs
Comment 9 Geoffrey Garen 2008-10-15 13:30:44 PDT
*** Bug 21616 has been marked as a duplicate of this bug. ***
Comment 10 Cameron Zwarich (cpst) 2008-10-15 13:43:52 PDT
Comment on attachment 24367 [details]
patch, finished product with no known bugs

r=me
Comment 11 Geoffrey Garen 2008-10-15 16:34:35 PDT
Committed revision 37622.

Darin mentioned in person that we might crash when starting debugging from within a modal dialog. Due to bug 21626, I wasn't able to verify if that was the case or not. So, for the time being, I've just added an ASSERT with an early return to cover the possibility.