EntityNode is bloaty

VERIFIED FIXED in mozilla0.9.2

Status

()

P3
normal
VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

Trunk
mozilla0.9.2
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

EntityNode (in htmlparser/src/nsHTMLEntity.cpp) is currently 92 bytes each - 4
bytes for the unicode codepoint and 4 bytes for the nsCAutoString.  This can be
reduced to 8 bytes since the strings are already stored in the text segment of
the program so there's no need to copy them into nsCAutoStrings.  The patch
reduces copying both at the time of construction of the table and at the time of
comparison, although there are further performance improvements that could be
made if we need to.

Patch coming.  I still need to figure out how to test the caller of
UnicodeToEntity.  EntityToUnicode clearly works since pages with character
entities display fine.

Harish, could you review?
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9.2
(Assignee)

Comment 2

18 years ago
I just changed the comment in nsHTMLEntities.h to reflect the change:
- * returns an empty string if the entity cannot be mapped. 
+ * returns null if the entity cannot be mapped. 
(Assignee)

Comment 3

18 years ago
s/already stored in the text segment of the program/already stored in the text
segment of the program or, for short-lived EntityNode objects, provided by the
creator/

Comment 4

18 years ago
See also bug 48855.

Comment 5

18 years ago
Patch looks good. r=harishd

Comment 6

18 years ago
+static const char* gEntityNames[] = {

Last I read, there's a preference that this be const char* const, no?

Other than that, sr=vidur.
(Assignee)

Comment 7

18 years ago
OK, added the const to gEntityNames ("const char* const") and the parallel const
("const PRInt32") for the gEntityCodes array.

And, FWIW, I tested the conversion the other way using the editor (by typing in
the characters וילר� and making sure the entities came out the right way),
which, from looking at the code, seems to be the only user of the reverse
conversion.

Updated

18 years ago
Blocks: 83989

Comment 8

18 years ago
a= asa@mozilla.org for checkin to the trunk.
(on behalf of drivers)
(Assignee)

Comment 9

18 years ago
Fix checked in 2001-06-04 17:39 PDT.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 10

18 years ago
Marking verified as per above developer comments.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.