Created attachment 369431 [details] [diff] [review] fixes bug Symbian folks are dying for this. runtime flag so its easily testable, always compiling and so mac 10.4 and lower can turn it off.
Attachment #369431 - Flags: review?(lhansen)
Comment on attachment 369431 [details] [diff] [review] fixes bug Reason for rejection: Patch does not apply cleanly to current redux, there's a big block that doesn't fit in GCHeap::Decommit. Another comment: Are you sure it's a good idea to burden every platform under the sun with having to implement (even stubs for) VM support when there's no gain? I know MacOS 10.4 is wonky but I don't know why everyone else should suffer because of that.
Attachment #369431 - Flags: review?(lhansen) → review-
The patch is based on having https://bugzilla.mozilla.org/attachment.cgi?id=369421&action=edit already applied. Right now there's only one platform that doesn't use VM support (Symbian) at all so I didn't think it was a big deal. Also for other platforms there's theories floating around that our VM fanciness doesn't buy us that much, this will enable us to test those theories.
Still rejected, GCHeapGeneric.cpp is missing a definition osSupportsVirtualMemory, suggesting it's not been tested at all. Other than that the code looks all right.
Hmm, nice catch. We don't have any builds that use GCHeapGeneric, I'll think of something...
Created attachment 369504 [details] [diff] [review] new patch Swapped in GCHeapGeneric.cpp on mac and tested it. Lots of problems with it. Need to figure out how to build/test it regularly (configure switch?).
Comment on attachment 369504 [details] [diff] [review] new patch No obvious bugs. For shell builds we could at least compile the generic heap as part of every build on one or all desktop platforms and have a switch to link against it or not; this would require the platform interface to be abstracted out, which is desirable anyway IMO.
Attachment #369504 - Flags: review?(lhansen) → review+
I'm landing Rishit's patch next and a lot of these static GCHeap methods will become VMPI functions, should be a little cleaner.
pushed as 1632
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Resolved fixed engineering / work item that has been pushed. Setting status to verified.
Status: RESOLVED → VERIFIED
Is there a build configuration that QE should work into the buildbot system? (re: comment #6).
(In reply to comment #10) > Is there a build configuration that QE should work into the buildbot system? > (re: comment #6). GCHeapGeneric is gone. As for the run-time switch it is tested when we're testing OSX 10.4 - it has some quirks and we don't use the VM functions but fall back on malloc/free instead.
You need to log in before you can comment on or make changes to this bug.