Closed
Bug 112470
Opened 23 years ago
Closed 23 years ago
Make MOZ_TIMELINE and MOZ_TRACE_MALLOC default
Categories
(SeaMonkey :: Build Config, defect, P1)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.9
People
(Reporter: dp, Assigned: dp)
Details
Attachments
(3 files, 3 obsolete files)
2.52 KB,
patch
|
cathleennscp
:
review+
alecf
:
superreview+
|
Details | Diff | Splinter Review |
495 bytes,
patch
|
Details | Diff | Splinter Review | |
1.40 KB,
patch
|
netscape
:
superreview+
|
Details | Diff | Splinter Review |
Making these default will eliminate barrier to perf and footprint work. a debug build (default build) should turn these ON by default. These should be individually ON/OFF able. (for linux look at leak-webshell-count to see how to do it right - dbaron)
Assignee | ||
Updated•23 years ago
|
Severity: normal → major
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.7
Assignee | ||
Comment 1•23 years ago
|
||
Things to check / fix: - ensure trace_malloc doesn't cost unless enabled - MOZ_TIMELINE should not print by default. NS_TIMELINE_ENABLE should be required for it to print.
Assignee | ||
Updated•23 years ago
|
Priority: -- → P1
Assignee | ||
Comment 2•23 years ago
|
||
TIMELINE outputs wont be printed by default. NS_TIMELINE_ENABLE would be required. To check this in mcafee needs to set that for tinderboxes.
Assignee | ||
Comment 3•23 years ago
|
||
xpcom/base/makefile.win was the only makefile using MOZ_TRACE_MALLOC and I didnt of any other way for making it pick up the trace malloc setting. rules.mak was included after setting all of EXPORTS, C_OBJS and CPP_OBJS and that was when we needed the setting. If this is too ugly, I could include all the files that for specific to trace malloc unconditionally and ifdef the files toally out in their source. But then that would still get this one file nsTraceMalloc.h into include which I didnt' like. Hence the hack.
Attachment #59878 -
Attachment is obsolete: true
Comment 4•23 years ago
|
||
Umm, attachment 59955 [details] [diff] [review] looks like an updated version of attachment 59878 [details] [diff] [review]. I don't see the xpcom/base/makefile.win changes that you're talking about. What's the ETA on the unix & mac changes?
Assignee | ||
Comment 5•23 years ago
|
||
Comment on attachment 59955 [details] [diff] [review] Windows: MOZ_TIMELINE and MOZ_TRACE_MALLOC on for debug builds Seawood, the xpcom/base/makefile.win patch is at the top of this attachment. Maybe your cache was playing funny games with you. Unix is monday. mac - I need simons help. I hope to try get it in by next weekend.
Assignee | ||
Comment 6•23 years ago
|
||
- should I checkin configure or configure.in I vaguely remember I should change configure.in and checking it in will cause configure to change automatically. For now the patch contains configure changed. - makes sure --disable-timeline or --disable-trace-malloc is obeyed as override - only for debug timeline and tracemalloc turned ON
> - should I checkin configure or configure.in I vaguely remember I should change
> configure.in and checking it in will cause configure to change automatically.
> For now the patch contains configure changed.
Check in configure.in, and configure will be updated automatically within 15
minutes.
Assignee | ||
Comment 8•23 years ago
|
||
same as patch 59955. With diff -u
Attachment #59955 -
Attachment is obsolete: true
Attachment #60222 -
Flags: review+
Assignee | ||
Comment 9•23 years ago
|
||
Ccing Garrett as he is also touching this area to get trace-malloc working on optimized builds on windows.
Comment 10•23 years ago
|
||
Comment on attachment 60222 [details] [diff] [review] windows: timeline and trace malloc enabled default for debug builds r=cathleen
Comment 11•23 years ago
|
||
Comment on attachment 60068 [details] [diff] [review] unix: timeline and trace_malloc enabled default for debug builds r=blythe
Attachment #60068 -
Flags: review+
Assignee | ||
Comment 12•23 years ago
|
||
As part of this, should we also pull/build space-trave mozilla/tools/trace-malloc by default ? I think it would make developers life easy. What do others think ?
Comment 13•23 years ago
|
||
I agree that they will need the tools build in order to use the default functionality. they build really quick from a time standpoint.
Comment 14•23 years ago
|
||
I agree to pull/build mozilla/tools/trace-malloc. otherwise, once you generate the data from running the build, you would still need to pull/build the tool to analyze that big chuck of data.
Comment 15•23 years ago
|
||
Comment on attachment 60222 [details] [diff] [review] windows: timeline and trace malloc enabled default for debug builds wait, there is a FAR easier way to fix xpcom/base/makefile.win I'll attach a patch momentarily however, sr=alecf on the rest
Attachment #60222 -
Flags: superreview+
Attachment #60222 -
Flags: needs-work+
Comment 16•23 years ago
|
||
Assignee | ||
Comment 17•23 years ago
|
||
Alec, excellent. That works. I will adopt that in instead of my hack. Thanks.
Assignee | ||
Comment 18•23 years ago
|
||
Here is the configure.in patch.
Attachment #60068 -
Attachment is obsolete: true
Comment 19•23 years ago
|
||
Comment on attachment 60411 [details] [diff] [review] Unix: MOZ_TIMELINE and TRACE_MALLOC ON by default for DEBUG builds sr=cls
Attachment #60411 -
Flags: superreview+
Comment 20•23 years ago
|
||
this is causing redness on shrike linux (among others)... i'm not sure what needs to be done to fix it.
Assignee | ||
Comment 21•23 years ago
|
||
Here is why : mozilla/Makefile.in has
< ifdef NS_TRACE_MALLOC
< tier_9_dirs += tools/trace-malloc
< endif
---
> #ifdef NS_TRACE_MALLOC
> #tier_9_dirs += tools/trace-malloc
> #endif
This is the patch from my tree (that I didn't include with my configure.in
patch!!!).
But then, this probably would break some of the tinderboxes building trace
malloc tools and getting bloatblame numbers. Hence I decided to back my change out.
Solution would be to pull tools/trace-malloc by default. We wanted to do that
anyway. Guess we need to do that along with the configure.in change.
Assignee | ||
Updated•23 years ago
|
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Assignee | ||
Comment 22•23 years ago
|
||
We did this one windows. We didn't do this on unix. I dont belive a lot of folks are using this. So I am going to call it quits and move on.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•