Currently, Vector<StackFrame> are being copied around multiple times in the process of creating Error stacks. We can do better.
Created attachment 281106 [details] proposed patch.
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.
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.
Thanks for the review. Landed in r201976: <http://trac.webkit.org/r201976>.