Closed Bug 231199 Opened 21 years ago Closed 21 years ago

Make mapped-attribute hash case sensitive


(Core :: DOM: Core & HTML, defect)

Not set





(Reporter: sicking, Assigned: sicking)



(Keywords: perf)


(1 file)

Rigth now the hash for 'uniqued' mapped attributes is case insensitive wrt the
values of the attributes. This is good and all in theory since it allows us to
squeeze more attribute-maps together and thus have a smaller rule-tree and fewer
attribute-maps allocated.

However, the disadvantage of having a case insensitive hash is that it's slower
then a case sensitive one. Having a case sensitive one would also make it easier
to use the same code for SVG and possibly XUL. And we'd be able to save some
code. There are also some attributes in html that should be case sensitive (like
background) so if we want to keep the current code we need to fix it anyway.

And it'll speed up the cases where we don't have a lot of elements with the same
attributes since hashing case sensitivly is faster the case insensitivly. I
would think this is a pretty common case too although i don't have any data to
back that up.

And in practice making the hash case sensitive probably won't cause a noticable
difference in number of unique maps for a page. In most cases where there are a
lot of elements with the same attributes the markup is generated either using
some GUI designer (composer, frontpage) or generated through CGI. In both cases
it's pretty likly that the same casing will be used everywhere. And even for
manually created pages i doubt that one and the same page will use more then two
or three different casings.

This also raises the issue of weather we should sort the mapped attributes
(which we do to make the hash attribute-order-insensitive). However the gain
seems a lot smaller here so before anyone does this we should probably
investigate weather we'd actually win anything.

As a sidenote i should mention that the current hash is actually partly
case-sensitive. The code to calculate hash-values is case sensitive but the code
to check equality is not. The endresult is that we most of the time already are
case sensitive. This has been the case since unique attributes were introduced.
Attached patch patch to fixSplinter Review
I also made the atomlist be sensitive to order of atoms. For class-attributes
this could increase the number of attribute-maps, however the class-attribute
isn't mapped so we won't have to worry about that. This way there won't be any
bugs if we use atomarrays for mapped attributes in the future.
Attachment #140232 - Flags: superreview?(jst)
Attachment #140232 - Flags: review?(peterv)
Attachment #140232 - Flags: review?(peterv) → review+
Comment on attachment 140232 [details] [diff] [review]
patch to fix

Attachment #140232 - Flags: superreview?(jst) → superreview+
Keywords: perf
patch checked in yesterday. Thanks for reviews
Closed: 21 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.