81746 - EntityNode is bloaty

Status: VERIFIED FIXED

(Core :: DOM: HTML Parser, defect, P3)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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?

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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. 

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

[rbs]

See also bug 48855.

Comment 5

[harishd]

Patch looks good. r=harishd

Comment 6

[vidur (gone)]

+static const char* gEntityNames[] = {

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

Other than that, sr=vidur.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 8

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in 2001-06-04 17:39 PDT.

Comment 10

[bsharma]

Marking verified as per above developer comments.