Matt, I tested this on Firefox and Chrome and saw that Chrome captures up to 200 frames. I don’t see Firefox capturing frames at all when not throwing an Error. Were you looking at Error.stack for Firefox when you came up with the 128 frames number? Maybe there’s a Firebug option I’m not familiar with?
Geoff, It just occurred to me that the developer does have one recourse: he/she can use break on exception thrown / on uncaught exception, and inspect the full stack via the debugger. Mark > On Mar 28, 2017, at 9:08 PM, Mark Lam <[email protected]> wrote: > > Currently, there’s no way for the developer to change this. We can certainly > make it an option that the Inspector can change if desired. > > Mark > > >> On Mar 28, 2017, at 7:35 PM, Matt Baker <[email protected] >> <mailto:[email protected]>> wrote: >> >>> On Mar 28, 2017, at 4:23 PM, Geoffrey Garen <[email protected] >>> <mailto:[email protected]>> wrote: >>> >>> Does the separate exceptionStackTraceLimit mean that if a developer gets a >>> truncated stack trace in the Web Inspector, there’s no way for the >>> developer to remedy that? Is that what other browsers’ developer tools do? >> >> FireFox and Chrome show console entires with exception stack traces with 128 >> and 200 frames (respectively). >> >>> >>> Geoff >>> >>>> On Mar 28, 2017, at 4:09 PM, Mark Lam <[email protected] >>>> <mailto:[email protected]>> wrote: >>>> >>>> To follow up, I’ve implemented the change in r214289: <http://trac.webkit >>>> <http://trac.webkit/>.org/r214289>. Error.stackTraceLimit is now 100. I >>>> also implemented a separate exceptionStackTraceLimit for stack traces >>>> captured at the time of throwing a value (not to be confused with >>>> Error.stack which is captured at the time of instantiation of the Error >>>> object). exceptionStackTraceLimit is also limited to 100 by default. >>>> >>>> Mark >>>> >>>> >>>>> On Mar 17, 2017, at 1:04 PM, Mark Lam <[email protected] >>>>> <mailto:[email protected]>> wrote: >>>>> >>>>> @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 <[email protected] >>>>>> <mailto:[email protected]>> wrote: >>>>>> >>>>>>> >>>>>>> On Mar 17, 2017, at 11:09 AM, Mark Lam <[email protected] >>>>>>> <mailto:[email protected]>> 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 <[email protected] >>>>>>>> <mailto:[email protected]>> 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 <[email protected] >>>>>>>>> <mailto:[email protected]>> 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 >>>>>>>>> [email protected] <mailto:[email protected]> >>>>>>>>> https://lists.webkit.org/mailman/listinfo/webkit-dev >>>>>>>>> <https://lists.webkit.org/mailman/listinfo/webkit-dev> >>>>>>>> >>>>>>> >>>>>>> _______________________________________________ >>>>>>> webkit-dev mailing list >>>>>>> [email protected] <mailto:[email protected]> >>>>>>> https://lists.webkit.org/mailman/listinfo/webkit-dev >>>>>>> <https://lists.webkit.org/mailman/listinfo/webkit-dev> >>>>> _______________________________________________ >>>>> webkit-dev mailing list >>>>> [email protected] <mailto:[email protected]> >>>>> https://lists.webkit.org/mailman/listinfo/webkit-dev >>>>> <https://lists.webkit.org/mailman/listinfo/webkit-dev> >>>> _______________________________________________ >>>> webkit-dev mailing list >>>> [email protected] <mailto:[email protected]> >>>> https://lists.webkit.org/mailman/listinfo/webkit-dev >>>> <https://lists.webkit.org/mailman/listinfo/webkit-dev> >>> >>> _______________________________________________ >>> webkit-dev mailing list >>> [email protected] <mailto:[email protected]> >>> https://lists.webkit.org/mailman/listinfo/webkit-dev >>> <https://lists.webkit.org/mailman/listinfo/webkit-dev> >> > > _______________________________________________ > webkit-dev mailing list > [email protected] > https://lists.webkit.org/mailman/listinfo/webkit-dev
_______________________________________________ webkit-dev mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-dev
