Thanks for the detailed write-up. The main thing that sticks out to me in this data is that Safari defaults to capturing a stack that is, in the worst case, roughly 3000X larger than the stack in IE and Chrome. That’s a big difference. I think this could be a real website compatibility problem since an author who tested in IE or Chrome might not notice a repeatedly thrown exception, which could cause time or memory or even bandwidth problems (if the author phoned home with exception data) in Safari.
Since Chrome and IE have already adopted it, I like Error.stackTraceLimit, and I think we should propose standardizing it. (We can standardize Error.stackTraceLimit even before we fully standardize the text content of Error.stack.) I think 10 is possibly an unnecessarily low default. Most stack traces are bigger than 10. 30 would still be two orders of magnitude better than the status quo. Even 50 or 100 might be OK, as long as your testing shows it’s not too expensive. Elipsizing was a cool idea, but given the behavior of other browsers, I think it’s better to truncate Error.stack and consider elipsizing in Web Inspector, where presentation matters more and has less of an effect on web compatibility. Geoff > On Mar 17, 2017, at 11:09 AM, Mark Lam <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. > > 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 >> >
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
