Summary: | Web Inspector: don't push script sources to frontend if scripts panel is inactive | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Pavel Podivilov <podivilov> | ||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | joepeck, pfeldman, pmuellr, timothy, yurys | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
Pavel Podivilov
2010-06-09 07:33:23 PDT
Pushing only scripts metadata and requesting script source lazily from the front end would reduce traffic between InspectorController and the front end. It would improve remote debugger performance so both engines should benefit from that. Another option would be in case of v8 to push already compiled scripts only when scripts panel is shown or when debugger hits a breakpoint. Any objections to the first approach? Created attachment 58378 [details]
Patch
Comment on attachment 58378 [details]
Patch
WebCore/inspector/InspectorBackend.h:94
+ void resolveScriptSource(long callId, const String& sourceID);
getScriptSource? There is not much to resolve there...
WebCore/inspector/InspectorController.cpp:1698
+ String dataToSend = ScriptDebugServer::shared().isDebuggerAlwaysEnabled() ? "" : data;
Can we make it always lazy regardless the setting?
Created attachment 58381 [details]
Patch
(In reply to comment #3) > (From update of attachment 58378 [details]) > WebCore/inspector/InspectorBackend.h:94 > + void resolveScriptSource(long callId, const String& sourceID); > getScriptSource? There is not much to resolve there... > Done. > WebCore/inspector/InspectorController.cpp:1698 > + String dataToSend = ScriptDebugServer::shared().isDebuggerAlwaysEnabled() ? "" : data; > Can we make it always lazy regardless the setting? Done. Now it's lazy no matter what kind of debugger is used. I had some concerns about slowing things down in Safari. Created attachment 58384 [details]
Patch
Comment on attachment 58384 [details]
Patch
This is scary!
Comment on attachment 58384 [details] Patch Clearing flags on attachment: 58384 Committed r60965: <http://trac.webkit.org/changeset/60965> All reviewed patches have been landed. Closing bug. Comment on attachment 58384 [details] Patch This looks like a good solution! Nice! I know this landed already (Sorry I'm so far behind). There were some typos in the comments, nothing too important. Just keep an eye out for these in future patches. > + script content lazily at the moment it should be displyed. It is critical for Typo: s/displyed/displayed/; (this is in each ChangeLog) > + // Don't send script content to the front end until it's realy needed. Typo: s/realy/really/; |