Bug 16561

Summary: remove debugger overhead from non-debugged JavaScript execution
Product: WebKit Reporter: Darin Adler <darin>
Component: JavaScriptCoreAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, ggaren, mjs, oliver
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
patch
none
improved patch; 1.022x as fast SunSpider eric: review+

Description Darin Adler 2007-12-21 11:48:39 PST
I got a 1.5% speed-up by putting the debugger code into separate nodes, only created if needed.
Comment 1 Darin Adler 2007-12-21 12:29:40 PST
Created attachment 18037 [details]
patch
Comment 2 Darin Adler 2007-12-21 12:56:14 PST
Created attachment 18038 [details]
improved patch; 1.022x as fast SunSpider
Comment 3 Oliver Hunt 2007-12-21 13:31:52 PST
Is it possible to break out the sourceelements change into a seperate patch? it makes the current patch somewhat noisy.
Comment 4 Eric Seidel (no email) 2007-12-21 13:40:31 PST
Comment on attachment 18038 [details]
improved patch; 1.022x as fast SunSpider

Looks great.

Two things:
1.  SourceElements::release( needs a better name:
children->release(m_children);
that code makes little sense at first pass.

2nd.  This now changes the behavior of Drosera so that you need to attach *before* you load a page.  We need to file at least one (possibly two) followup bugs to this one.
1.  Add the ability walk the tree and swap in/out break nodes for when the first/last debugger attaches/detaches.
2.  If #1 doesn't get done soon, we'll need to teach Drosera how to recognize that it's trying to debug a JavaScript page which hasn't been parsed with debugging information and display a note to the user to reload the scripts (or just have it reload the scripts automatically)
Comment 5 Darin Adler 2007-12-21 13:56:05 PST
(In reply to comment #4)
> This now changes the behavior of Drosera so that you need to attach
> *before* you load a page.

We just checked with Tim Hatcher, and it turns out that was already true about Drosera.

r28937