@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
