1045239 - Add a version of nsTHashtable::SizeOfExcludingThis and SizeOfIncludingThis that uses the entry's SizeOfExcludingThis

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Andrew McCreight [:mccr8]]

Right now, to call SizeOfExcludingThis on nsTHashtable you have to define a custom callback method, which is pretty ridiculous, as it is a static property of the type of the entry in many situations. This patch defines a new version that calls SizeOfExcludingThis on the entry, and defines such a method for nsCharPtrHashKey.

Comment 1

[Eric Rahm [:erahm]]

Good idea! Vaguely relates to this: bug 1006667

I don't see the patch attached yet, how would we distinguish b/w the situation where we don't want to measure the entries and where we just want to use a default "call my SizeOf function" wrapper?

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Add a version of nsTHashtable::SizeOfExcludingThis that uses the entry's SizeOfExcludingThis.

I'm not entirely sure what you are asking. The patch just adds a new overload. The existing behavior remains in place if people call it with more args.

Comment 3

[Eric Rahm [:erahm]]

(In reply to Andrew McCreight [:mccr8] from comment #2)
> Created attachment 8463609 [details] [diff] [review]
> Add a version of nsTHashtable::SizeOfExcludingThis that uses the entry's
> SizeOfExcludingThis.
> 
> I'm not entirely sure what you are asking. The patch just adds a new
> overload. The existing behavior remains in place if people call it with more
> args.

Ah ok, an overload, for some reason I thought we were using a default functor (I was confusing that with the default user arg). My only comment is that there should be a |nsTHashtable::SizeOfIncludingThis| version as well.

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: Add a version of nsTHashtable::SizeOfExcludingThis that uses the entry's SizeOfExcludingThis.

This patch does not actually use this yet.  I have a later one that uses it.

I can file a followup bug if you'd like to audit the existing places where we call nsTHashtable::SizeOfExcludingThis to see if we can use it more.

try: https://tbpl.mozilla.org/?tree=Try&rev=4ccdf4dd033a

Comment 5

[Nathan Froyd [:froydnj]]

Comment on attachment 8464155 [details] [diff] [review]
Add a version of nsTHashtable::SizeOfExcludingThis that uses the entry's SizeOfExcludingThis.

Review of attachment 8464155 [details] [diff] [review]:
-----------------------------------------------------------------

A followup bug would be wonderful; I'm sure there's a number of places where we can use this.

Comment 6

[Andrew McCreight [:mccr8]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d1056f293ddd

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d1056f293ddd