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)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla1.2alpha
People
(Reporter: dbaron, Assigned: dbaron)
Details
(Whiteboard: [patch][whitebox])
Attachments
(1 file)
9.23 KB,
patch
|
sicking
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
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...
Assignee | ||
Comment 1•22 years ago
|
||
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [patch]
Target Milestone: --- → mozilla1.2alpha
Comment 2•22 years ago
|
||
Comment on attachment 85759 [details] [diff] [review] patch sr=bzbarsky
Attachment #85759 -
Flags: superreview+
Attachment #85759 -
Flags: review+
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
Assignee | ||
Comment 4•22 years ago
|
||
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.
Assignee | ||
Comment 5•22 years ago
|
||
Fix checked in, 2002-08-17 05:17 PDT.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Updated•21 years ago
|
Whiteboard: [patch] → [patch][whitebox]
You need to log in
before you can comment on or make changes to this bug.
Description
•