@Geoff, my testing shows that we can do 200 frames and still perform well (~1 
second to console.log Error.stack).  Base on what we at present, I think 100 is 
a good round number to use as our default stackTraceLimit.

> On Mar 17, 2017, at 11:40 AM, Maciej Stachowiak <m...@apple.com> wrote:
> 
>> 
>> On Mar 17, 2017, at 11:09 AM, Mark Lam <mark....@apple.com 
>> <mailto:mark....@apple.com>> wrote:
>> 
>> 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:
>> 
>> 1. Chrome
>>     number of frames captured: 10
>>     length of e.stack string: 824 chars
>>     time to console.log e.stack: 0.27 seconds
>> 
>> 2. Firefox
>>     number of frames captured: 129
>>     length of e.stack string: 8831 chars
>>     time to console.log e.stack: 0.93 seconds
>> 
>> 3. Safari
>>     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.
>> 
>> I wasn’t aware of the Error.stackTraceLimit, but that does sound like a 
>> better solution than my proposal since it gives developers the ability to 
>> capture more stack frames if they need it.  Chrome’s default 
>> Error.stackTraceLimit appears to be 10.  MS appears to support it as well 
>> and defaults to 10 
>> (https://docs.microsoft.com/en-us/scripting/javascript/reference/stacktracelimit-property-error-javascript
>>  
>> <https://docs.microsoft.com/en-us/scripting/javascript/reference/stacktracelimit-property-error-javascript>).
>>   Firefox does now.
> 
> Out of curiosity: Why does Firefox capture 129 frames instead of 31722 in 
> this case? Do they have a hardcoded limit?

Actually, my previous frame counts are a bit off.  I was using 
e.stack.split(/\r\n|\r|\n/).length as the frame count.  Below, I just copy the 
console.log dump into an editor and take the line count from there as the frame 
count instead.  The result of that string.split appears to be a bit off from 
the actual frames printed by console.log. 

I also modified my recursing test function to console.log the re-entry count on 
entry and this is what I saw:

1. Chrome
    test reported reentry count = 10150
    ....split(…).length = 11 (because Chromes starts e.stack with a line 
"RangeError: Maximum call stack size exceeded”)
    e.stack lines according to editor = 10 frames

2. Firefox
    test reported reentry count = 222044
    ....split(…).length = 129 (probably because there’s an extra newline in 
there somewhere)
    e.stack lines according to editor = 128 frames

3. Safari
    test reported reentry count = 31701
    ....split(…).length = 31722 (I don’t know why there’s a 21 frame 
discrepancy here.  I’ll debug this later)
    e.stack lines according to editor = ??? frames (WebInspector hangs every 
time I try to scroll in it, let alone let me highlight and copy the stack 
trace.  So I gave up)

Assuming the test function frame is not significantly different in size for all 
browsers, it looks like:
1. Chrome uses a much smaller stack (about 1/3 of our stack).
2. Firefox uses a much larger stack (possibly the full machine stack), but caps 
its Error.stack to just 128 frames (possibly a hardcoded limit).

Mark


> 
>  - Maciej
> 
>> 
>> Does anyone object to us adopting Error.stackTraceLimit and setting the 
>> default to 10 to match Chrome?
>> 
>> Mark
>> 
>> 
>> 
>>> On Mar 16, 2017, at 11:29 PM, Geoffrey Garen <gga...@apple.com 
>>> <mailto:gga...@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?
>>> 
>>> We already limit the size of the JavaScript stack to avoid performance 
>>> problems like the ones you mention in many other contexts. Why is that 
>>> limit not sufficient?
>>> 
>>> Did you consider implementing Chrome’s Error.stackTraceLimit behavior?
>>> 
>>> Geoff
>>> 
>>>> On Mar 16, 2017, at 10:09 PM, Mark Lam <mark....@apple.com 
>>>> <mailto:mark....@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:
>>>> 
>>>>     foo10001
>>>>     foo10000
>>>>     foo9999
>>>>     foo9998
>>>>     …
>>>>     foo1
>>>>     foo0
>>>> 
>>>> 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?
>>>> 
>>>> Thanks.
>>>> 
>>>> Mark
>>>> 
>>>> 
>>>> 
>>>> _______________________________________________
>>>> webkit-dev mailing list
>>>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>>>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>>>> <https://lists.webkit.org/mailman/listinfo/webkit-dev>
>>> 
>> 
>> _______________________________________________
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>> <https://lists.webkit.org/mailman/listinfo/webkit-dev>
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Reply via email to