Jesse Ruderman Reporter
	Description • 16 years ago

I suspect the weirdness in bug 482236 is a result of more than 2^32 objects being created.  Can we fix this by replacing some "PRInt32" and "nsrefcnt" with 64-bit types?

http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsTraceRefcntImpl.cpp#122

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 1 • 16 years ago

Yes, I think we need to. I agree that this is quite likely the cause of random orangeness.

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 2 • 16 years ago

Should be very easy ... any volunteers?

	Serge Gautherie (:sgautherie)
	Comment 3 • 16 years ago

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1237584473.1237590737.1535.gz
OS X 10.5.2 mozilla-central unit test on 2009/03/20 14:27:53

	Serge Gautherie (:sgautherie)
	Comment 4 • 16 years ago

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1237853880.1237861341.4729.gz
OS X 10.5.2 mozilla-central unit test on 2009/03/23 17:18:00

	David Baron :dbaron: Assignee
	Comment 6 • 16 years ago

So, I actually wrote another patch for this before finding this bug.  I think my patch is roughly a subset of the above changes (except I converted some things to unsigned as well), and I think it's the subset that we want.  I don't think we need to change the reference counts for individual objects; we only need to change the variables used to accumulate totals.

I think we definitely do *not* want to use "long nsrefcnt".

	David Baron :dbaron: Assignee
	Comment 8 • 16 years ago

http://hg.mozilla.org/mozilla-central/rev/374ed6b2fd1a

	Serge Gautherie (:sgautherie)
	Comment 9 • 16 years ago

(In reply to comment #6)
> except I converted some things to unsigned as well

Then, this can't handle 'Adds < Releases' or 'Creates < Destroys' anymore, can it ?

	David Baron :dbaron: Assignee
	Comment 10 • 16 years ago

(In reply to comment #9)
> Then, this can't handle 'Adds < Releases' or 'Creates < Destroys' anymore, can
> it ?

If either of those are the case, we've probably crashed already.  (Or the logging is broken.)

Those cases were already pretty broken even before this patch, though.

	Serge Gautherie (:sgautherie)
	Comment 11 • 16 years ago

(In reply to comment #10)
> If either of those are the case, we've probably crashed already.  (Or the
> logging is broken.)

Well, I wonder about such a crash,
and (bug 456894 comment 0 and) your bug 482236 comment 7 even made me think that to detect such cases was a feature.
(That's why I wanted to mention it.)

> Those cases were already pretty broken even before this patch, though.

But if you say now that the new behavior is better, then fine by me.
(In which case, I'll update (back?) the python parsing code.)

	David Baron :dbaron: Assignee
	Comment 12 • 15 years ago

Comment on attachment 371921 [details] [diff] [review]
smaller patch

I think we want this on 1.9.1 to fix bug 483917

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 13 • 15 years ago

Comment on attachment 371921 [details] [diff] [review]
smaller patch

a191=beltzner

	David Baron :dbaron: Assignee
	Comment 14 • 15 years ago

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/49aec8ec91ca

	Shriram (irc: Mavericks)
	Comment 15 • 15 years ago

Oh ,yes.
we definitely don't want the "nsrefcnt".
I apologize for adding it and I think I wasn't sure whether PRUint64 could in fact replace "long nsrefcnt", even though I spent quite a bit of time trying to locate the appropriate replacement for nsrefcnt.
Apart from that I wasn't really sure about adding the unsigned part for the resolution.
For some reason, it looks like I didn't CC this to myself back then :(

Thanks sgautherie, dbaron and beltzner for the resolution of this bug
(In reply to comment #6)
> So, I actually wrote another patch for this before finding this bug.  I think
> my patch is roughly a subset of the above changes (except I converted some
> things to unsigned as well), and I think it's the subset that we want.  I don't
> think we need to change the reference counts for individual objects; we only
> need to change the variables used to accumulate totals.
> 
> I think we definitely do *not* want to use "long nsrefcnt".

	Shriram (irc: Mavericks)
	Comment 16 • 15 years ago

#if defined(XP_WIN) && PR_BYTES_PER_LONG == 4
typedef unsigned long nsrefcnt;
#else
typedef PRUint32 nsrefcnt;
#endif
@ 
http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nscore.h
 helped me justify the usage of PRUint64.