Bug 158664 – Minimize the amount of memcpy done for allocating Error stacks.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

Currently, Vector<StackFrame> are being copied around multiple times in the process of creating Error stacks.  We can do better.

[Mark Lam]

Created attachment 281106 [details]
proposed patch.

[Darin Adler]

Comment on attachment 281106 [details]
proposed patch.

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

> Source/JavaScriptCore/inspector/ScriptCallStackFactory.cpp:140
> +    const Vector<StackFrame>& stackTrace = exception->stack();

I would prefer this:

    auto& stackTrace = exception->stack();

What do you think?

> Source/JavaScriptCore/interpreter/StackVisitor.h:192
> +class StackDepthCountFunctor {
> +public:
> +    StackVisitor::Status operator()(StackVisitor&) const
> +    {
> +        count++;
> +        return StackVisitor::Continue;
> +    }
> +    
> +    mutable size_t count { 0 };
> +};

I’m not sure this belongs in a header. Why not put it closer to where it’s used? Maybe it could even be a lambda that captures a reference to a local variable.

[Mark Lam]

Comment on attachment 281106 [details]
proposed patch.

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

>> Source/JavaScriptCore/inspector/ScriptCallStackFactory.cpp:140
>> +    const Vector<StackFrame>& stackTrace = exception->stack();
> 
> I would prefer this:
> 
>     auto& stackTrace = exception->stack();
> 
> What do you think?

Sure.  I'm fine with that.

>> Source/JavaScriptCore/interpreter/StackVisitor.h:192
>> +};
> 
> I’m not sure this belongs in a header. Why not put it closer to where it’s used? Maybe it could even be a lambda that captures a reference to a local variable.

Thanks.  I'm so used to doing this the old way that I missed that we can use a lambda here.  I put this functor class in this header because I think it can be useful for other uses other than just the current one e.g. in custom code added during debugging sessions.  Rather than having to roll one every time, I thought I'd just put it here as a utility.  But since the lambda I would have to write instead for this is almost trivial, I'll go with the lambda instead.

[Mark Lam]

Thanks for the review.  Landed in r201976: <http://trac.webkit.org/r201976>.