Bug 21735 - Emit profiling instrumentation only if the Web Inspector's profiling feature is enabled
Summary: Emit profiling instrumentation only if the Web Inspector's profiling feature ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
: 21296 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-10-18 16:18 PDT by Geoffrey Garen
Modified: 2008-11-24 17:09 PST (History)
1 user (show)

See Also:


Attachments
patch (45.54 KB, patch)
2008-10-18 16:50 PDT, Geoffrey Garen
no flags Details | Formatted Diff | Diff
patch - removed the comment tim mentioned (45.53 KB, patch)
2008-10-18 17:55 PDT, Geoffrey Garen
darin: 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-18 16:18:58 PDT
Patch coming.
Comment 1 Geoffrey Garen 2008-10-18 16:50:28 PDT
Created attachment 24493 [details]
patch
Comment 2 Timothy Hatcher 2008-10-18 17:30:22 PDT
Comment on attachment 24493 [details]
patch

             // Profiler check
             emitGetCTIParam(CTI_ARGS_profilerReference, X86::eax);

You should remove that comment too.

Maybe recompileAllJSFunctionsSoon() should be on some other class or standalone function? Or moved to JSCore? It seems weird for it to be on the JavaScriptDebugServer now that it is used be the profiler.
Comment 3 Geoffrey Garen 2008-10-18 17:54:55 PDT
(In reply to comment #2)
> (From update of attachment 24493 [details] [edit])
>              // Profiler check
>              emitGetCTIParam(CTI_ARGS_profilerReference, X86::eax);
> 
> You should remove that comment too.

Fixed!

> Maybe recompileAllJSFunctionsSoon() should be on some other class or standalone
> function? Or moved to JSCore? It seems weird for it to be on the
> JavaScriptDebugServer now that it is used be the profiler.

recompileAllJSFunctionsSoon() can't move to JSCore unless we move the Timer class down to WTF, too.

It can't be standalone unless we rework the Timer class to support standalone functions in addition to member functions.

Maybe these changes would be worth it, though.

Ideally, I think we'd use a shared object like GCController. JavaScriptDebugServer seemed like that object to me. It's not the profiler, but it makes sense to me that it knows how to recompile things, and that the profiler / Settings object can use it for that purpose.

But maybe I should just split off a new object -- CompilerController?
Comment 4 Geoffrey Garen 2008-10-18 17:55:58 PDT
Created attachment 24496 [details]
patch - removed the comment tim mentioned

Updated patch to remove the comment Tim mentioned.
Comment 5 Darin Adler 2008-10-18 23:06:58 PDT
Comment on attachment 24496 [details]
patch - removed the comment tim mentioned

r=me
Comment 6 Timothy Hatcher 2008-10-23 13:34:30 PDT
Geoff landed this in r37730.

http://trac.webkit.org/changeset/37730
Comment 7 Cameron Zwarich (cpst) 2008-11-24 17:09:40 PST
*** Bug 21296 has been marked as a duplicate of this bug. ***