Closed
Bug 673197
Opened 13 years ago
Closed 13 years ago
Enable jemalloc on VC8/9 express
Categories
(Core :: Memory Allocator, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: neil, Assigned: neil)
References
Details
Attachments
(3 files)
4.39 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
915 bytes,
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
1.09 KB,
patch
|
Details | Diff | Splinter Review |
Currently jemalloc requires VC8/9 pro or VC10. It should be possible to enable new-style jemalloc on VC8/9 express too.
Assignee | ||
Comment 1•13 years ago
|
||
This patch searches for the VC8/9 source. Only if it doesn't find it does it use the new-style jemalloc. This means that it doesn't affect Tinderbox. Because of the crtdll check, this actually works on debug VC8/9 express builds and debug and opt Windows SDK 6/7/7.1 x86/x64 builds too. (Well, SDK 6 is currently broken for other reasons...)
Comment on attachment 547466 [details] [diff] [review] Proposed patch [Checked in: See comment 13] Awesome stuff.
Attachment #547466 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 3•13 years ago
|
||
Pushed changeset 83ef35b794ce to mozilla-central.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 4•13 years ago
|
||
Backed out: http://hg.mozilla.org/mozilla-central/rev/a627b24e684e
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 5•13 years ago
|
||
vc10 jemalloc build has been collapsed after back out.
Updated•13 years ago
|
Attachment #549626 -
Flags: review?(Ms2ger)
Comment 6•13 years ago
|
||
if use -MANIFEST:NO, vc8/9 build crashes. vc10 is no probrem without -MANIFEST:NO.
Assignee | ||
Comment 7•13 years ago
|
||
(In reply to comment #6) > if use -MANIFEST:NO, vc8/9 build crashes. > vc10 is no probrem without -MANIFEST:NO. vc8/9 build shouldn't be using that line.
Assignee | ||
Comment 8•13 years ago
|
||
Comment on attachment 549626 [details] [diff] [review] fix back out misstake [Checked in: Comment 11] Oops, thanks for spotting this!
Attachment #549626 -
Flags: review?(Ms2ger) → review+
Comment 9•13 years ago
|
||
(In reply to comment #7) > (In reply to comment #6) > > if use -MANIFEST:NO, vc8/9 build crashes. > > vc10 is no probrem without -MANIFEST:NO. > > vc8/9 build shouldn't be using that line. sorry, vc8/9 **express** jemalloc build crashes.
Comment 10•13 years ago
|
||
(In reply to comment #8) > Comment on attachment 549626 [details] [diff] [review] [diff] [details] [review] > fix back out misstake. > > Oops, thanks for spotting this! thanks. this patch solves Bug 674972.
Assignee | ||
Comment 11•13 years ago
|
||
(In reply to comment #10) > (In reply to comment #8) > > Oops, thanks for spotting this! > > thanks. > this patch solves Bug 674972. Pushed changeset f92e021f1f44 to mozilla-central. (In reply to comment #9) > (In reply to comment #7) > > (In reply to comment #6) > > > if use -MANIFEST:NO, vc8/9 build crashes. > > > vc10 is no probrem without -MANIFEST:NO. > > > > vc8/9 build shouldn't be using that line. > > sorry, vc8/9 **express** jemalloc build crashes. Ah, I realise my mistake, the crtdll.obj bug doesn't affect debug builds, but release builds have the problem. Please can you file a new bug on vc8/9 express release builds? You'll need to ask khuey to review your patch.
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
Comment 12•13 years ago
|
||
(In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #8) > > > Oops, thanks for spotting this! > > > > thanks. > > this patch solves Bug 674972. > > Pushed changeset f92e021f1f44 to mozilla-central. > thanks. > (In reply to comment #9) > > (In reply to comment #7) > > > (In reply to comment #6) > > > > if use -MANIFEST:NO, vc8/9 build crashes. > > > > vc10 is no probrem without -MANIFEST:NO. > > > > > > vc8/9 build shouldn't be using that line. > > > > sorry, vc8/9 **express** jemalloc build crashes. > > Ah, I realise my mistake, the crtdll.obj bug doesn't affect debug builds, > but release builds have the problem. > > Please can you file a new bug on vc8/9 express release builds? You'll need > to ask khuey to review your patch. ok. I filed a new Bug 675519.
Updated•13 years ago
|
Attachment #549626 -
Attachment description: fix back out misstake. → fix back out misstake
[Checked in: Comment 14]
Updated•13 years ago
|
Attachment #549626 -
Attachment description: fix back out misstake
[Checked in: Comment 14] → fix back out misstake
[Checked in: Comment 11]
Updated•13 years ago
|
Attachment #547466 -
Attachment description: Proposed patch → Proposed patch
[Backed out: Comment 4]
Attachment #547466 -
Attachment is obsolete: true
Comment 13•13 years ago
|
||
Comment on attachment 547466 [details] [diff] [review] Proposed patch [Checked in: See comment 13] Oh, a modified version of this patch was eventually checked in: http://hg.mozilla.org/mozilla-central/rev/7bc488fc53a3
Attachment #547466 -
Attachment description: Proposed patch
[Backed out: Comment 4] → Proposed patch
[Checked in: See comment 13]
Updated•13 years ago
|
Attachment #547466 -
Attachment is obsolete: false
You need to log in
before you can comment on or make changes to this bug.
Description
•