If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

comment out nsTraceRefcnt in optimized builds

RESOLVED WONTFIX

Status

()

Core
XPCOM
P1
normal
RESOLVED WONTFIX
17 years ago
8 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

Trunk
Future
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [xptest])

Attachments

(4 attachments)

As Brendan said in bug 61243, we should make class nsTraceRefcnt completely
#ifdef NS_BUILD_REFCNT_LOGGING (which is defined by default in debug builds
but not optimized builds).  I propose also that we pull WalkTheStack out of
nsTraceRefcnt and make it it's own global function, depending on a different
#ifdef (since I don't think it's #ifdef-ed at all now, and there is at least
one other user).  Perhaps it should be renamed to NS_WalkTheStack and within
|#if defined(DEBUG) || defined(NS_BUILD_REFCNT_LOGGING)|?
And I meant to assign this to myself...
Assignee: scc → dbaron
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla0.9
Created attachment 25134 [details] [diff] [review]
mostly completed patch (hand-edited diff that won't apply)
Cool -- how do you want to land this?

Once it's in, I'll try to unify nsTraceMalloc's stack and symbol stuff with it.

/be
Created attachment 25439 [details] [diff] [review]
patch for this bug and bug 14989 and bug 63665 (ignore makefile changes removing nslogging stuff)
Created attachment 25440 [details] [diff] [review]
The real changes in the large chunk that was moved from nsTraceRefcnt.cpp to nsStackWalker.cpp in the above patch

Comment 6

17 years ago
Looks ok. Why not make LoadLibrarySymbols and DemangleSymbol accessable from C,
too? (e.g., I wish that the Boehm GC would dump demangled symbols...)

r=waterson
LoadLibrarySymbols is called from the function that loads the DLL, i.e., the
code in xcDll.cpp.  DemangleSymbol might be nice from the boehm GC, except,
at least on Linux, it leaks, which leads to exponential growth of the leak
report if you do real-time symbol demangling. :-(

I thought about making them accessible from C, too, but I didn't see how they
would be useful (see above).  Do you think I should anyway?  (If so, can you
think of good names for them?)
Is anybody interested in testing this on Windows/Mac?  I'd normally be
comfortable landing something like this on a quiet Sunday morning, but I'm
not going to be able to find Mac help then...  Or do you think it's safe
enough?
Since I really need to get the stuff in bug 66159 tested on Windows and Mac,
moving this to mozilla 0.9.1
Target Milestone: mozilla0.9 → mozilla0.9.1
Whiteboard: [xptest]
Pushing out again.  I'll try to land this early in 0.9.2.  Really.  (I should
have time this time around.)
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Priority: P2 → P1
Created attachment 41141 [details] [diff] [review]
patch, brought up to date
Target Milestone: mozilla0.9.3 → mozilla1.0
What I discovered (a month or two ago) was wrong with this is that it didn't
account for the windows library loading functions properly.  Probably more of
that would need to be in nsStackWalker, or something.
Keywords: mozilla1.0
This should probably be done before 1.0 for binary compatibility, but I'm not
going to be able to do it.
Target Milestone: mozilla1.0 → Future

Comment 14

16 years ago
I do not think that there is a problem with binary compatiblity if one builds
with optimized and does not set FORCE_BUILD_REFCNT_LOGGING.  Is this a correct
observation?
That's correct, but we might in the future want to be able to change the
signatures of methods in nsTraceRefcnt, and right now, we can't (for any clients
that link to xpcom -- although I really think they shouldn't be).

Comment 16

16 years ago
Sure we can change the symbol.  I agree with you... clients that link against
symbols that are not frozen shouldn't.

Updated

12 years ago
QA Contact: kandrot → nobody
QA Contact: nobody → xpcom

Comment 17

8 years ago
we now provide these symbols in release builds as we should, I think (although the stacks are not always so useful)
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.