[webkit-dev] Proposal to limit the size of the captured exception stack
mark.lam at apple.com
Fri Mar 17 11:09:34 PDT 2017
Thanks for the reminder to back observations up with data. I was previously running some tests that throws StackOverflowErrors a lot (which tainted my perspective), and I made a hasty conclusion which isn’t good. Anyway, here’s the data using an instrumented VM to take some measurements and a simple test program that recurses forever to throw a StackOverflowError (run on a MacPro):
1. For a release build of jsc shell:
Time to capture exception stack = 0.002807 sec
Number of stack frames captured = 31722
sizeof StackFrame = 24
total memory consumed = ~761328 bytes.
2. For a debug build of jsc shell:
Time to capture exception stack = 0.052107 sec
Number of stack frames captured = 31688
sizeof StackFrame = 24
total memory consumed = ~760512 bytes.
So, regarding performance, I was wrong. The amount of time taken to capture the entire JS stack each time is insignificant.
Regarding memory usage, ~760K is not so good, but maybe it’s acceptable.
Comparing browsers with their respective inspectors open:
number of frames captured: 10
length of e.stack string: 824 chars
time to console.log e.stack: 0.27 seconds
number of frames captured: 129
length of e.stack string: 8831 chars
time to console.log e.stack: 0.93 seconds
number of frames captured: 31722
length of e.stack string: 218821 chars
time to console.log e.stack: 50.8 seconds
4. Safari (with error.stack shrunk to 201 frames at time of capture to simulate my proposal)
number of frames captured: 201
length of e.stack string: 13868 chars
time to console.log e.stack: 1 second
With my proposal, the experience of printing Error.stack drops from 50.8 seconds to about 1 second. The memory used for capturing the stack also drops from ~760K to 5K.
Does anyone object to us adopting Error.stackTraceLimit and setting the default to 10 to match Chrome?
> On Mar 16, 2017, at 11:29 PM, Geoffrey Garen <ggaren at apple.com> wrote:
> Can you be more specific about the motivation here?
> Do we have any motivating examples that will tell us wether time+memory were unacceptable before this change, or are acceptable after this change?
> In our motivating examples, does Safari use more time+memory than other browsers? If so, how large of a stack do other browsers capture?
> Did you consider implementing Chrome’s Error.stackTraceLimit behavior?
>> On Mar 16, 2017, at 10:09 PM, Mark Lam <mark.lam at apple.com> wrote:
>> Hi folks,
>> Currently, if we have an exception stack that is incredibly deep (especially for a StackOverflowError), JSC may end up thrashing memory just to capture the large stack trace in memory. This is bad for many reasons:
>> 1. the captured stack will take a lot of memory.
>> 2. capturing the stack may take a long time (due to memory thrashing) and makes for a bad user experience.
>> 3. if memory availability is low, capturing such a large stack may result in an OutOfMemoryError being thrown in its place.
>> The OutOfMemoryError thrown there will also have the same problem with capturing such a large stack.
>> 4. most of the time, no one will look at the captured Error.stack anyway.
>> Since there isn’t a standard on what we really need to capture for Error.stack, I propose that we limit how much stack we capture to a practical size. How about an Error.stack that consists of (1) the top N frames, (2) an ellipses, and (3) the bottom M frames? If the number of frames on the stack at the time of capture is less or equal to than N + M frames, then Error.stack will just show the whole stack with no ellipses. For example, if N is 4 and M is 2, the captured stack will look something like this:
>> If we pick a sufficient large number for N and M (I suggest 100 each), I think this should provide sufficient context for debugging uses of Error.stack, while keeping an upper bound on how much memory and time we throw at capturing the exception stack.
>> My plan for implementing this is:
>> 1. change Exception::finishCreation() to only capture the N and M frames, plus possibly 1 ellipses placeholder in the between them.
>> 2. change all clients of Exception::stack() to be able to recognize and render the ellipses.
>> Does anyone object to doing this or have a compelling reason why this should not be done?
>> webkit-dev mailing list
>> webkit-dev at lists.webkit.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the webkit-dev