> 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.
Out of curiosity: Why does Firefox capture 129 frames instead of 31722 in this case? Do they have a hardcoded limit? - 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 >> > > _______________________________________________ > webkit-dev mailing list > 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