| Summary: | Emit profiling instrumentation only if the Web Inspector's profiling feature is enabled | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Geoffrey Garen <ggaren> | ||||||
| Component: | JavaScriptCore | Assignee: | Geoffrey Garen <ggaren> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | zwarich | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Mac | ||||||||
| OS: | OS X 10.5 | ||||||||
| Attachments: |
|
||||||||
|
Description
Geoffrey Garen
2008-10-18 16:18:58 PDT
Created attachment 24493 [details]
patch
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.
(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? Created attachment 24496 [details]
patch - removed the comment tim mentioned
Updated patch to remove the comment Tim mentioned.
Comment on attachment 24496 [details]
patch - removed the comment tim mentioned
r=me
Geoff landed this in r37730. http://trac.webkit.org/changeset/37730 |