Closed
Bug 3412
Opened 26 years ago
Closed 24 years ago
[PP]GWorld allocations should be smarter
Categories
(Core :: Layout, defect, P2)
Tracking
()
VERIFIED
FIXED
Future
People
(Reporter: sfraser_bugs, Assigned: sfraser_bugs)
References
Details
(Keywords: perf, Whiteboard: [nsbeta3-] Partial fix in hand)
When the rendering context creates a GWorld, it allocates it in the heap.
These GWorlds can be very large (1+ Mb), and heap allocations can fail, say
if the heap is fragmented. There should be some fallbacks, as follows:
1. Try to allocate GWorld in heap.
2. If that fails, try to allocate using temp mem (useTempMem flag)
3. If that fails, the view mgr (or whoever) should fall back to drawing
on screen.
nsRenderingContextMac::CreateDrawingSurface() should also nil out the
returned surface in case of failure, and in case the caller ignores the
return value (as the view mgr was -- bug filed).
Comment 1•26 years ago
|
||
With the current API, we can't set the 'useTempMem' flag in order to allocate the
offscreen buffer in temp memory because the pixels have to stay locked until they
are disposed (and InsideMac says that 'useTempMem' should always be used in
conjunction with AllowPurgePixels so that other apps can launch). This should
change when the new API (nsIDrawingSurface) is implemented because it contains
methods to lock and unlock the pixels.
With the new API, setting the returned drawing surface to nil in case of error
will be no good because the application will likely consider it as a
nsIDrawingSurface and call some of its methods. If it's nil, it will break too:
the application must test for error codes.
Updated•26 years ago
|
Target Milestone: M3
Updated•26 years ago
|
Status: NEW → ASSIGNED
Updated•26 years ago
|
Target Milestone: M4 → M5
Comment 2•26 years ago
|
||
Changed target to M5
Summary: GWorld allocations should be smarter → [PP]GWorld allocations should be smarter
Updated•26 years ago
|
Target Milestone: M5
Updated•26 years ago
|
Target Milestone: M8
Updated•26 years ago
|
Target Milestone: M8 → M9
Comment 3•26 years ago
|
||
Puhing my Mac-specific bugs to M9 before reassigning them to someone else.
Comment 4•26 years ago
|
||
Reassigned to dcone
Updated•26 years ago
|
Status: NEW → ASSIGNED
Updated•26 years ago
|
Target Milestone: M9 → M12
Updated•26 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 26 years ago
Resolution: --- → FIXED
Comment 5•26 years ago
|
||
GWorlds are allocated in the nsDrawingSurface code, using the ::NewGWorld()
call. Errors are returned from this call if the GWorld fails to be created.
Failure to on create the offscreen buffer is handled in layout, and onscreen
does kick in if the offscreen creation fails.
Assignee | ||
Updated•26 years ago
|
Status: RESOLVED → REOPENED
Assignee | ||
Comment 6•26 years ago
|
||
Reopening. The part that needs to be smarter is GWorld allocation in
nsDrawingSurfaceMac :: Init(). We call NewGWorld() to make a GWorld in the
heap, but if this fails, then we should try with the useTempMem flag to make
one in temp mem. This is really important, because we make some very large
GWorlds.
Assignee | ||
Updated•26 years ago
|
Resolution: FIXED → ---
Updated•26 years ago
|
Status: REOPENED → RESOLVED
Closed: 26 years ago → 26 years ago
Resolution: --- → FIXED
Comment 7•26 years ago
|
||
The offscreen memory can be kept around for awhile, so using this flag is a
no-no. According the Inside mac, only use this memory if it is going to be used
for a fleeting moment.
Assignee | ||
Comment 8•26 years ago
|
||
Using temp mem for GWorlds is common practice. IM says also not to leave handles
locked in temp mem (which is really the same issuse), yet Communicator, MRJ and
many other apps do this all the time. I think using temp mem for GWorlds is far
more acceptable that falling back on on-screen drawing.
Comment 9•26 years ago
|
||
Why don't we both look into this issue, I will do some more snooping you can to
and we can talk about this on the phone next week and really nail this.
Updated•26 years ago
|
Resolution: FIXED → ---
Comment 10•26 years ago
|
||
Simon is right on that one: we should use temp mem if possible. As described
above, the strategy should be:
1. Try to allocate GWorld in heap.
2. If that fails, try to allocate using temp mem (useTempMem flag)
3. If that fails, the view mgr (or whoever) should fall back to drawing
on screen.
Reopening, CCing myself and marking 12672 as dependent.
Comment 11•26 years ago
|
||
I don't understand what is meant by "common practice". Are you telling me that
inside MAC is incorrect.. that is is ok to use temp memory for long periods of
time. I am not saying that you are wrong, I just want to know why its ok to do
what the documention say not to do. Is there a Tech note or an article on this
subject? Currently I don't feel comfortable about using something that is
temporary on a permenent basis (at least while our app is running). The
offscreen buffer will use this memory for as long as the program runs if it uses
it.
Assignee | ||
Comment 12•26 years ago
|
||
"Common practice" describes patterns of use which may somestimes go against the
recommendations of Inside Mac, but which are known to be in use in a number of
widespread applications and system services.
Remember that Inside Mac is several years out of date, and often makes
recommendations, like this one, which were appropriate in older system versions,
but which no longer apply. In particular, IM:Memory recommends not leaving
handles in temp mem locked over calls to WaitNextEvent becaues of problems on
older systems. This is no longer such a problem.
Comment 13•26 years ago
|
||
MSIE even caches the bitmaps of the previous pages to speed up the Back/Forward
operations...
On another note, I think that we should change the order of the operations above
to:
1) allocate in temp mem
2) if it fails, allocate in the heap
That way, we'll be less likely to run out of memory in other modules and we'll
look better if we can lower the memory partition.
Comment 14•25 years ago
|
||
Running the debug build, I was getting short in memory in the past few days so,
instead of increasing the application partition even more, I changed
nsDrawingSurfaceMac to allocate the pixels with 'useTempMem'.
Updated•25 years ago
|
Status: REOPENED → RESOLVED
Closed: 26 years ago → 25 years ago
Resolution: --- → FIXED
Comment 15•25 years ago
|
||
Added code to use temp memory if the GWorld can not be allocated using normal
allocation.
Updated•25 years ago
|
Status: RESOLVED → VERIFIED
Comment 16•25 years ago
|
||
Based on Don's comments, marking this verified fixed in the Sept 16th build.
Comment 17•25 years ago
|
||
I'm reopening this bug in connection with bug 20743 and especially the comments
from Simon (2000-06-07 10:46) and Cathy (2000-06-19 14:36).
I suggest we first try to allocate the gworlds in temp mem, and use the heap only
if there isn't any memory. In the current code, we try the heap first, then the
temp mem. See my comments from 1999-09-03 11:52 in the present bug.
Comment 18•25 years ago
|
||
I am going to give this one to Simon Fraser.. he can change the order if he
thinks this will reduce the memory problems and that it is really important for
bug 20743. If I have this bug I am going to move this to future because it
falls under the wire for my present bug list.
Assignee: dcone → sfraser
Status: REOPENED → NEW
Comment 19•25 years ago
|
||
this is more of a code level correctness versus feature correctness, this will
improve stability under low memeory conditions
Target Milestone: M17 → M18
Assignee | ||
Comment 21•25 years ago
|
||
I'm very reluctant to allocate GWorlds in tempMem if we leave them locked all the
time. dcone: why can't we lock and unlock the GWorld pixels in
nsDrawingSurfaceMac::Lock()/nsDrawingSurfaceMac::Unlock()? If that was true, then
the view manager could lock and unlock the offscreen buffer appropriately.
Status: NEW → ASSIGNED
Assignee | ||
Comment 23•25 years ago
|
||
Over to dcone to see if we can fix the GWorld locking situation.
Assignee: dcone
Status: ASSIGNED → NEW
Updated•25 years ago
|
Status: NEW → ASSIGNED
Comment 25•25 years ago
|
||
This bug has been marked future because we have determined that it is not
critical for netscape 6.0. If you feel this is an error, or if it blocks your
work in some way -- please attach your concern to the bug for reconsideration.
Target Milestone: M18 → Future
Comment 26•24 years ago
|
||
i have the changes for allocating the GWorlds out of temp mem in my tree, what do
we want to do with them?
Whiteboard: [nsbeta3-] → [nsbeta3-] Partial fix in hand
Comment 27•24 years ago
|
||
I would talk with Simon/Pierre.. if this modification is really important for
the Mac memory stuff then get permission from PDT to check this in..
Comment 28•24 years ago
|
||
You just changed all this.. correct Simon. I will give this to you so you can
disposition this.
Assignee: dcone → sfraser
Status: ASSIGNED → NEW
Assignee | ||
Comment 29•24 years ago
|
||
This is done now for GWorlds created by the Rendering Context. We still suck on
the lock/unlock front, but I think a separate bug exists for that.
Status: NEW → RESOLVED
Closed: 25 years ago → 24 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•