Closed Bug 148368 Opened 22 years ago Closed 22 years ago

convert mapped attribute table to pldhash

Categories

(Core :: CSS Parsing and Computation, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla1.2alpha

People

(Reporter: dbaron, Assigned: dbaron)

Details

(Whiteboard: [patch][whitebox])

Attachments

(1 file)

I was looking at a profile for the big callers of nsHashtable::Get, and the
mapped attribute table was #3 in the profile (where #1 and #2 were the
already-changed-to-pldhash RuleHash (bug 112318) and the binding manager's
binding table).

So, patch to convert the mapped attribute table to pldhash (which should be a
bit of memory savings, too) coming up...
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [patch]
Target Milestone: --- → mozilla1.2alpha
Comment on attachment 85759 [details] [diff] [review]
patch

sr=bzbarsky
Attachment #85759 - Flags: superreview+
Comment on attachment 85759 [details] [diff] [review]
patch

+  memset(entry, 0, sizeof(MappedAttrTableEntry));

are you allowed to clear out the entire entry including the PLDHashEntryHdr
part? Wouldn't entry->mAttributes = nsnull do?

in HTMLStyleSheetImpl::Reset
+  PL_DHashTableFinish(&mMappedAttrTable);
+  mMappedAttrTable.ops = nsnull;
do you need an if (mMappedAttrTable.ops) around that?

in HTMLStyleSheetImpl::UniqueMappedAttributes
+  if (!mMappedAttrTable.ops) {
+    PL_DHashTableInit(&mMappedAttrTable, &MappedAttrTable_Ops, nsnull,
+		       sizeof(MappedAttrTableEntry), 16);
   }
you should check if this succeeds


 HTMLStyleSheetImpl::DropMappedAttributes(nsIHTMLMappedAttributes* aMapped)
 {
+  NS_ENSURE_TRUE(aMapped, NS_OK);

if it's valid to pass null as aMapped then you should probably use 
if (!aMapped)
  return NS_OK;
instead since NS_ENSURE warns.


r=sicking
The |memset| is what the default ClearEntry op does, and is needed to maintain
the invariant that all table entries are zero-initialized.

I had that null-check in Reset in my tree already -- I probably put it in just
after writing the patch because it crashed, and then forgot to reattach the patch.

I added a success-check to the PL_DHashTableInit call and return
NS_ERROR_OUT_OF_MEMORY with a null out parameter.

It's not (IMO) valid to pass null, but the old code allowed it, so I changed it
to an assertion with NS_OK return.
Fix checked in, 2002-08-17 05:17 PDT.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Whiteboard: [patch] → [patch][whitebox]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: