Bug 116753

Summary: Remove Interpreter::retrieveLastCaller().
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, ggaren, mhahnenberg, msaboff, oliver
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
the patch ggaren: review+

Description Mark Lam 2013-05-24 20:28:53 PDT
Reduce the number of stack walking functions, and we're starting by removing Interpreter::retrieveLastCaller().
Comment 1 Mark Lam 2013-05-24 21:15:46 PDT
Created attachment 202873 [details]
the patch
Comment 2 Mark Lam 2013-05-24 21:44:20 PDT
I've run the riun-javascriptcode-tests and the layout tests for:
    fast/js
    fast/regex
    ietestcenter/JavaScript
    sputnik

No new failures were seen.  The remaining layout tests are still slowly cooking.
Comment 3 Geoffrey Garen 2013-05-24 22:54:36 PDT
Comment on attachment 202873 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=202873&action=review

r=me

> Source/JavaScriptCore/API/JSContextRef.cpp:183
> +    for (Vector<StackFrame>::const_iterator iter = stackTrace.begin(); iter < stackTrace.end(); iter++) {

WebKit style prefers index over iterator in Vector iterations for a terse, easier-to-read code. (See http://www.webkit.org/coding/coding-style.html.)

> Source/JavaScriptCore/interpreter/Interpreter.h:250
> +        JS_EXPORT_PRIVATE void getStackTrace(Vector<StackFrame>& results, unsigned maxStackSize = INT_MAX);

Since this is an unsigned, it should be UINT_MAX.

> Source/WebCore/bindings/js/ScriptCallStackFactory.cpp:81
> +    exec->interpreter()->getStackTrace(stackTrace);

You can pass maxStackSize here, and remove the code to check it below.

> Source/WebCore/bindings/js/ScriptCallStackFactory.cpp:85
> +    Vector<StackFrame>::const_iterator iter = stackTrace.begin();
> +    iter++;
> +    for (; iter < stackTrace.end(); iter++) {
> +        frames.append(ScriptCallFrame(iter->friendlyFunctionName(exec), iter->friendlySourceURL(), iter->friendlyLineNumber()));

This case particularly benefits from index-style iteration, since index-style iteration doesn't need the dummy iter++ at the start.
Comment 4 Mark Lam 2013-05-25 13:23:21 PDT
Thanks for the review.  Addressed issues in Geoff's feedback.  Also fix 2 bugs in the patch:

1. In JSXMLHttpRequestCustom.cpp's JSXMLHttpRequest::send(), we need to explicitly specify size of the stackTrace Vector to guarantee that we have an entry at index 1.

2. In ScriptCallStackFactory.cpp's createScriptCallStack(), we need to increment maxStackSize by 1 before passing it to getStackTrace() since we're going to skip the 1st frame.

The updated patch has been tested with run-javascriptcore-tests and layout tests fast/js, fast/regex, ietestcenter/JavaScript, and sputnik.

Landed in r150690: <http://trac.webkit.org/changeset/150690>.