92576 - nsVoidArray -> nsAutoVoidArray, all but content/layout from 90545

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

All patches for nsVoidArray -> nsAutoVoidArray for modules other than content or
layout in bug 90545's second patch.  Assumes the primary patch from 90545 has
been applied.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

I'll take this.  module is really various, not xpcom.

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Attachment: patch for all but layout/content

Comment 3

[Johnny Stenback (:jst)]

Any reason to not make nsRendericContextGTK::mStateCache a nsAutoVoidArray and
not a nsAutoVoidArray*, that way you'd cut down on yet one more allocation per
rendering context? Same goes for RDFContentSinkImpl::mContextStack?

Other than that, sr=jst

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Sure, mStateCache should be a member var.  It's instantiated in the CTOR.

mContextStack I wasn't so sure about, because I didn't know how often we'd have
a push of context onto the stack.  If  we're pretty sure we always (usually) use
the context state, then no problem.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Update - mStateCache a member var, not a pointer

Comment 6

[Chris Waterson]

r=waterson on the changes in mozilla/rdf.

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

r= requests sent to darin, pavlov, kmcclusk, valeski, roc, and kin (for
different pieces of this patch).

Comment 8

[kinmoz]

Regarding the transaction manager changes ... we need to keep in mind that every 
text widget in the app (form/xul textfields, textareas, file input, and 
passwords), as well as all flavors of Composer, have their own transaction 
manager, and I believe all of them have only one listener registered.

Do we really want the extra 28 bytes overhead (7 unused pointer slots) for each 
widget and Composer?

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

Added these to the wrong bug:

	have r=darin on netwerk/http portion via email
	For uriloader portion via email: looks fine to me, r=darin

Kin writes:
>as well as all flavors of Composer, have their own transaction 
>manager, and I believe all of them have only one listener registered.
>
>Do we really want the extra 28 bytes overhead (7 unused pointer slots) for each 
>widget and Composer?

The overhead is 0 or negative (counting memory allocator overhead/rounding),
since currently all of those with their 1 listener have space for 8 allocated
(as soon as you add the first one, it allocates space for 8).

I'm strongly considering making another AutoVoidArray class that has a different
default (1); I can't make it specified easily due to the annoyances of C++ and
member var sizes.  I also am considering allowing initializing VoidArray with an
array of any size.  However, those are for the future.

Comment 10

[Kevin McCluskey (gone)]

r=[e-mail] for change to nsViewManager.h

Comment 11

[kinmoz]

r=[e-mail] for the TransactionManager changes.

Comment 12

[Randell Jesup [:jesup] (needinfo me)]

Has all reviews; will try to commit tonight

Comment 13

[Randell Jesup [:jesup] (needinfo me)]

Just checked, oops: we're waiting for Pav for the GFX review and then I can
check in.

Comment 14

[Stuart Parmenter]

other than the fact that the gfx/src/nsImage* files no longer exist, the gfx
part looks ok to me.  r=pavlov

Comment 15

[Randell Jesup [:jesup] (needinfo me)]

Fix checked in (minus the files that were removed)