> On Mar 16, 2017, at 10:09 PM, Mark Lam <[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.
How much? What would be the new footprint with your suggestion below? > 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. Web Inspector’s interface wouldn’t look too good with such a large call stack anyway, so this doesn’t degrade the current experience IMO. Maybe even 50+50 would be fine. I’ve never seen a call stack that large in JS code (but then again, I don’t use fancy frameworks). > 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] > https://lists.webkit.org/mailman/listinfo/webkit-dev _______________________________________________ webkit-dev mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-dev
