Closed
Bug 556382
Opened 14 years ago
Closed 14 years ago
Link 32-bit Windows builds with LARGEADDRESSAWARE
Categories
(Firefox Build System :: General, defect)
Tracking
(blocking2.0 betaN+)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: mozilla, Assigned: Mitch)
References
Details
Attachments
(1 file)
2.59 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.2) Gecko/20100316 Firefox/3.6.2 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.2) Gecko/20100316 Firefox/3.6.2 Microsoft continues to work to limit applications usage of more memory and now require applications to be compiled with a switch "IMAGE_LARGE_ADDRESS_AWARE", in order for a 32-bit application to be *able* to use more than 2GB. This issue is only important as long as FF remains 32-bit on 64-bit machines, as when FF goes 64-bit, the problem will go away naturally. However, the change NOW, is trivial: just adding a flag to the linker "/LARGEADDRESSAWARE" -- and FF will be "enabled". Users can make use of this via their OS-boot time options. ---- See option "User-mode virtual address space for each 32-bit process" under column "Limit in 64-bit Windows". http://msdn.microsoft.com/en-us/library/aa366778%28VS.85%29.aspx Reproducible: Always Steps to Reproduce: 1. FF uses only 2GB, if switch isn't set. 2. if linked with the switch, FF is *allowed* to use up to 4GB (in 64-bit windows). 3.
Is this something worth doing?
Status: UNCONFIRMED → NEW
Component: Preferences → Build Config
Ever confirmed: true
Product: Firefox → Core
QA Contact: preferences → build-config
Summary: link-switch@build time to allow 4GB addr usage: please add LARGEADDRESSAWARE to link in 32bit Win builds → Link 32 bit Windows builds with LARGEADDRESSAWARE
Comment 2•14 years ago
|
||
I'm not sure what the actual consequences are. There might need to be code auditing before we could flip it. (I'm also not clear that there's a huge upside here. Does anyone want their browser using >2GB of memory?)
Indeed. I'm tempted to WONTFIX.
As explained in bug 590674 this is an actual issue as FF4 OOM-crashes around 1.5GB under heavy load on a 64bit windows system instead of using the theoretically available 4GB. Since no 64bit release of FF4 planned yet for windows this might be a viable workaround.
Comment 5•14 years ago
|
||
Note that this isn't 2GB allocated but 2GB of _address_space_ in use. As in, we can get there due to fragmentation, random large memory mappings from someone, etc... We've had a number of reports of "Firefox crashes with OOM without actually using as much RAM as I have", and I think we do want to fix this for Gecko 2.0.
blocking2.0: --- → ?
Comment 6•14 years ago
|
||
We had a known leak of VM earlier in the cycle, but that's fixed. I really don't think this is serious enough to block a release.
blocking2.0: ? → -
Comment 7•14 years ago
|
||
> We had a known leak of VM earlier in the cycle, but that's fixed
Yes, but people are still commonly hitting crashes at 1.5GB of memory usage....
a) this still occurs with FF4b6, actually it happens more often due to increased memory usage. So it's probably unrelated to the memory leak b) it happens right after startup when restoring a large session (200+ tabs), so it's not something that slowly leaks over time. Right now i'm mostly sticking to FF3 since it uses less memory with the same amount of tabs and therefore does not run into this memory barrier. Just to emphasize: If i take my FF3 sessionrestore as-is (it uses around 1GB ram) and move it to a clean FF4b6 profile it shoots up to about 1.5GB of memory on startup and OOM-crashes.
Ah, and some other aspects that should be considered a) I'm aware that heavy tab usage is not the norm at the moment, but this may change since tabcandy is designed to make a large number of tabs more manageable b) Whenever i get an OOM crash the crash reporter says "there was an error submitting your crash report", which means that the frequency of these crashes and thus the severity of the issue may be underreported.
Comment 10•14 years ago
|
||
Firefox 4 using more memory should be a separate bug. I might block on that. But I'm not going to block on just adding an extra 1G of address space to the Firefox process.
Comment 11•14 years ago
|
||
We use more address space, obviously. For one thing, we're mapping graphics card memory via some of our hardware acceleration stuff and so forth.... That doesn't seem like something we should block on, to me, whereas increasing our address space to handle these new mappings does.
Comment 12•14 years ago
|
||
Are we doing that per-tab or per-window? It seems to me that keeping graphics card memory alive for hidden tabs is a bad tradeoff, if that's what we're doing.
Comment 13•14 years ago
|
||
I have no idea, nor whether we can even control it very well; ccing some graphics folks.
Comment 14•14 years ago
|
||
(In reply to comment #11) > We use more address space, obviously. For one thing, we're mapping graphics > card memory via some of our hardware acceleration stuff and so forth.... It's not just gfx i think. After some suggestions i changed to the following settings: image.mem.decodeondraw = true image.mem.discardable = true gfx.direct2d.disabled = true gfx.font_rendering.directwrite.enabled = false mozilla.widget.render-mode = 0 with these settings i still get the behavior described in comment #8 > Just to emphasize: If i take my FF3 sessionrestore as-is (it uses around 1GB > ram) and move it to a clean FF4b6 profile it shoots up to about 1.5GB of memory > on startup and OOM-crashes. Just as a comparison: FF3.6 + 270 tabs = 1 to 1.2GB FF4b4 + 170 tabs = 1.4GB+ => crash FF4b4 + 170 tabs = 1.2GB (with image.mem settings) FF4b6 + 170 tabs = 1.6GB (with image.mem settings + HW acceleration off) => crash
Comment 15•14 years ago
|
||
(In reply to comment #10) > Firefox 4 using more memory should be a separate bug. I might block on that. > But I'm not going to block on just adding an extra 1G of address space to the > Firefox process. on a 64bit system it would actually add 2GB of address space.
(In reply to comment #12) > Are we doing that per-tab or per-window? It seems to me that keeping graphics > card memory alive for hidden tabs is a bad tradeoff, if that's what we're > doing. We aren't. We are using more memory per window, though.
Comment 17•14 years ago
|
||
(In reply to comment #10) > But I'm not going to block on just adding an extra 1G of address space to the > Firefox process. (In reply to comment #11) > That doesn't seem like something we should block on, to me, > whereas increasing our address space to handle these new mappings does. Those are basically 2 different solutions to the same problem: OOM crash due to increased virtual address space use. So I'll file an additional bug about increased memory usage, make bug 590674 depend on it too and request blocking2.0 on it instead, which should cover both points of view. Which way it'll be resolved ultimately shouldn't matter as long as it gets done.
Comment 18•14 years ago
|
||
(In reply to comment #10) > Firefox 4 using more memory should be a separate bug. done: bug 598466
Comment 19•14 years ago
|
||
so... since nobody seems to be working on this i tried it myself. downloaded visual studio, applied editbin.exe /largeaddressaware <path to firefox>\firefox.exe And voila, firefox is using 2GB private or working set and 2.6GB virtual address space and no more OOM-crashes.
Comment 20•14 years ago
|
||
we should do this. i don't believe we have any code which does evil things to pointers and expects to only be in the lower 2gb of space. because of the additional stuff we map in, our usable space for normal operations has indeed shrunken relative to previous versions. there is no affect on systems which aren't 64bit or don't boot with the /3gb switch. so it's safe for "average cases". and it's the right path for bigger/newer systems. it's also cheap to enable (as comment 19 notes, although one can do it as part of the build process instead of later).
Comment 21•14 years ago
|
||
I've been using the nightlies for a month now and applied the /LARGEADDRESSAWARE switch every time it updates and it has been using > 3GB virtual address space without issues. So from my point of view it would seem reasonable to include this in the build process.
Comment 22•14 years ago
|
||
Everyone agrees we want this. It's only waiting for somebody to write the patch.
Assignee | ||
Comment 23•14 years ago
|
||
I can regularly reproduce crashes without this fix at ~1.2GB memory used by the process. With the fix, I stress tested it with hundreds of sites/pages open, and process memory usage went over 2.1GB. Firefox paused a lot and was generally sluggish but did not crash.
Assignee: nobody → mitchell.field
Status: NEW → ASSIGNED
Attachment #488740 -
Flags: review?(benjamin)
Assignee | ||
Updated•14 years ago
|
Severity: normal → major
Summary: Link 32 bit Windows builds with LARGEADDRESSAWARE → Link 32-bit Windows builds with LARGEADDRESSAWARE
Version: unspecified → Trunk
Comment on attachment 488740 [details] [diff] [review] Patch v1 Why are we testing for MSC_VER >= 1400? Isn't Visual Studio 2005 the minimum required version on trunk these days? r=me regardless.
Attachment #488740 -
Flags: review?(benjamin) → review+
(In reply to comment #20) > we should do this. i don't believe we have any code which does evil things to > pointers and expects to only be in the lower 2gb of space. > > because of the additional stuff we map in, our usable space for normal > operations has indeed shrunken relative to previous versions. > > there is no affect on systems which aren't 64bit or don't boot with the /3gb > switch. so it's safe for "average cases". and it's the right path for > bigger/newer systems. > > it's also cheap to enable (as comment 19 notes, although one can do it as part > of the build process instead of later). This might affect crappy binary extensions. That's not a reason not to do it IMO, but there will be >0 fallout.
Assignee | ||
Updated•14 years ago
|
blocking2.0: - → ?
blocking2.0: ? → betaN+
Comment 26•14 years ago
|
||
(In reply to comment #25) > This might affect crappy binary extensions. That's not a reason not to do it > IMO, but there will be >0 fallout. problems would only occur when the virtual address space exceeds the 2GB limit, in which case it would crash anyway.
Comment 27•14 years ago
|
||
technically such extensions could crash in "interesting" ways instead of harmless ways... but yeah, i'd like to see this in.
Assignee | ||
Updated•14 years ago
|
Attachment #488740 -
Flags: approval2.0?
Assignee | ||
Updated•14 years ago
|
Attachment #488740 -
Flags: approval2.0?
Assignee | ||
Comment 28•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/6957f4aa855f
Comment 29•14 years ago
|
||
(In reply to comment #24) > Why are we testing for MSC_VER >= 1400? Isn't Visual Studio 2005 the minimum > required version on trunk these days? Actually even VC6 supports -LARGEADDRESSAWARE anyway.
Comment 30•14 years ago
|
||
This patch breaks the compilation of libffi on our (Instantbird) Windows builder. (See http://buildbot.instantbird.org/builders/win32-nightly-default/builds/301/steps/compile/logs/stdio for the build log) This seems somewhat similar to bug 590996.
I wonder why we don't see that on m-c.
Comment 32•14 years ago
|
||
(In reply to comment #31) > I wonder why we don't see that on m-c. I don't know. Bug 590996 is not visible on m-c either (on the tinderbox I mean). I initially thought it was because we still build with VS2005 and m-c was built with a newer version, but Shaver said some tinderboxes run VS2005.
All of our 32 bit Windows builds are done with VS2005. Mitch, can we resolve this bug?
Assignee | ||
Comment 34•14 years ago
|
||
I was only waiting for the builds. Resolving as fixed.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•