If a called function is a FunctionImp but doesn't have a function name, it is displayed in the profiler as "(Anonymous Function)" but this is true for prototypes and many sites use prototypes for the majority of their development. It would be better if we had a way to display more useful information. <rdar://problem/5958825> JSProfiler: Find more descriptive information to display than (Anonymous Function)
Created attachment 23824 [details] Test case
Kevin and I spent some time discussing this today :D
Created attachment 31781 [details] patch Made AssignDotNode, PropertyNode, AssignResolveNode propagate the identifier to the child node if it is a FuncExprNode. The name is saved on the FuncBodyNode. The JSFunction::calculatedDisplayName will try to use that one only if InternalFunction::calculatedDisplayName() returns an empty String.
Comment on attachment 31781 [details] patch Have you run performance numbers on this patch? (adding a virtual call to every normal assignment scares me a little) and i am concerned about the increase in size to FunctionBodyNode, although i don't have a really good solution that would avoid it. In general the patch looks fine but i'm too tired to do a proper review.
(In reply to comment #4) > (From update of attachment 31781 [details] [review]) > Have you run performance numbers on this patch? (adding a virtual call to every > normal assignment scares me a little) and i am concerned about the increase in > size to FunctionBodyNode, although i don't have a really good solution that > would avoid it. In general the patch looks fine but i'm too tired to do a > proper review. > It should affect the parse only and I don't know if SunSpider catches the parser speed. I've pasted the Sunspider results on http://pastebin.com/m76a3ee44 ("From" is CL @45069 build with release and "To" is @45069 with this patch)
Results of WebKitTools/Scripts/run-sunspider on macosx: http://pastebin.com/m21b4f477
Comment on attachment 31781 [details] patch clearing review as i think i misread the perf numbers
A 1% regression isn't really something we can land :-/
Created attachment 31925 [details] patch with unlikely Added UNLIKELY to ifs. It seems to improve speed. ("From" is the unchanged code): http://pastebin.com/m3a17662d
Comment on attachment 31925 [details] patch with unlikely It doesn't propagate the name to the new body when the functions are recompiled.
Created attachment 31929 [details] patch fix - added setContextualName calls in JavaScriptDebugServer.cpp when it recompiles all the js functions; - changed getContextualName() to contextualName()
Are there fresh performance numbers on this? Does it still cause a 1% regression?
(In reply to comment #12) > Are there fresh performance numbers on this? Does it still cause a 1% > regression? "patch fix" performance result are at http://pastebin.com/m3a17662d. I've just added UNLIKELY in the last patch and it seems faster. I don't know how it works on Windows. At the moment when those script nodes are created the profiler might not be enabled yet, so this change cannot be activated only when the profiler is enabled. Is there any way to determine if the "Show develop menu in menubar" option is checked, so that the code could bypass the useless virtual call to isFuncExprNode?.
Comment on attachment 31929 [details] patch fix Have you actually tested performance? You can simply use the run-sunspider script on the commandline to test this; "run-sunspider --runs 30" should give you a stable number. "run-sunspider --runs 30 > baseTimes", etc for before and after numbers then "sunspider-compare-results baseTimes newTimes" will give you actual performance comparison
Comment on attachment 31929 [details] patch fix Please put this up for review again one performance has been tested.
We did this a while ago in another patch.