Closed
Bug 3989
Opened 26 years ago
Closed 23 years ago
Raptor uses too much stack space
Categories
(Core :: Layout, defect, P3)
Tracking
()
RESOLVED
INVALID
M6
People
(Reporter: pierre, Assigned: rickg)
Details
This bug is related to #3899 where the page on http://www.mozilla.org/party/1999/
caused the Mac to crash because of insufficient stack space. I bumped it to 128K
from 64K to fix #3899 but we may still find another page somewhere else that will
cause an overflow.
The Mozilla page has 6 or 7 nested tables. The stack is relatively deep but stays
within acceptable levels (let's say less than a hundred return addresses) so it
looks like we are allocating too much space for variables on the stack on each
level.
Rick: we should talk about this as a group; its a *BIG BIG BIG* deal to handle
this. nglayout is designed to use the stack heavily. Maybe the MAC'rs can just
set it to 256k or 512k and be done with it? I don't know...
Reporter | ||
Comment 2•26 years ago
|
||
We can increase the stack size if we want; it's not a problem. I reported the bug
because it seemed to me that 64K was a lot of space for the depth of the stack.
Maybe we are allocating some buffers on the stack somewhere.
Let's make this a topic at next weeks Raptor all hands. We certainly do use the
stack to extreme to keep heap allocations down. Maybe we just need to make a
pass through the code to clean up coder "laziness".
Reporter | ||
Comment 4•26 years ago
|
||
When displaying the Mozilla party page, the stack was showing the following
pattern within a relatively deep recursive loop. Each function is preceded by the
number of bytes it uses on the stack. The total was 11408 bytes for each level of
recursion.
144 nsBlockFrame::ReflowDirtyLines()
96 nsBlockFrame::ReflowLine()
464 nsBlockFrame::ReflowBlockFrame()
624 nsBlockReflowContext::ReflowBlock()
944 nsTableOuterFrame::Reflow()
192 nsContainerFrame::ReflowChild()
304 nsTableFrame::Reflow()
688 nsTableFrame::ResizeReflowPass1()
192 nsContainerFrame::ReflowChild()
224 nsTableRowGroupFrame::Reflow()
368 nsTableRowGroupFrame::ReflowMappedChildren()
192 nsContainerFrame::ReflowChild()
336 nsTableRowFrame::Reflow()
624 nsTableRowFrame::InitialReflow()
192 nsContainerFrame::ReflowChild()
496 nsTableCellFrame::Reflow()
192 nsContainerFrame::ReflowChild()
304 nsAreaFrame::Reflow()
4832 nsBlockFrame::Reflow()
----
11408
I fixed the obvious problem in nsBlockFrame::Reflow(): an object of more than 4Kb
was allocated on the stack.
We should identify the recursive loops throughout the code and check whether they
contain errors like this one. Another step would be to reduce the stack use in
less obvious cases: in the example above, several functions use more than 600
bytes and one is at 900 bytes. Even after my fix, the total is at 7K for each
level of recursion.
Reporter | ||
Comment 5•26 years ago
|
||
I had to back out the fix in nsBlockFrame::Reflow because of performance
degradation in deeply nested tables. Instead, I increased the stack size in
Viewer and AppRunner to 256K. Hopefully that will be enough.
Reporter | ||
Comment 6•26 years ago
|
||
>>On the related bug report #3899, Simon Fraser asked:
Pierre, is jacking up the stack size the right way to go? What about embedding
applications -- this will be yet another thing they have to worry about, and
that will only bite them after a few users start crashing hard.
>>My response was:
On that particular problem, Kipp improved the stack use a little bit: the 4Kb
object was reduced to 1.7. Still, when we reflow deeply nested tables, each level
of recursion takes about 9K on the stack and it appears that overall we use much
more than (9Kb * number of nested levels).
I opened last week a separate bug report (#3989) to address the general problem
of stack space, not just this particular crash. As you said, increasing the stack
size shouldn't be the only solution. It hurts the memory footprint because we
must be rather conservative in the estimated size and, as you mentionned, it is
one more constraint for embedding apps.
Problem: as Kipp wrote, Raptor is designed to use the stack heavily. The issue
will be debated tomorrow. I added you to the CC list in bug #3989.
Status: ASSIGNED → RESOLVED
Closed: 26 years ago
Resolution: --- → REMIND
Moving to remind. A few of us are working on a plan to reduce overall memory
usage.
Updated•26 years ago
|
Status: RESOLVED → VERIFIED
Comment 8•26 years ago
|
||
Verifying as a Reminder.
Comment 10•23 years ago
|
||
Perf issues are handled in other bugs.
Status: REOPENED → RESOLVED
Closed: 26 years ago → 23 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•