Bug 71012 – Use StringHasher to generate the matched declaration cache hash

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Antti Koivisto]

It is faster and better than the current custom function.

[Antti Koivisto]

Created attachment 112672 [details]
patch

[Kenneth Rohde Christiansen]

Comment on attachment 112672 [details]
patch

r=me

[Antti Koivisto]

http://trac.webkit.org/changeset/98573

[Darin Adler]

Comment on attachment 112672 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=112672&action=review

> Source/WebCore/css/CSSStyleSelector.cpp:2139
> +    return StringHasher::hashMemory(declarations, sizeof(MatchedStyleDeclaration) * size);

Is there a possibility that this will hash uninitialized memory between the fields of the MatchedStyleDeclaration structure or between elements of the array?

[Antti Koivisto]

(In reply to comment #4)
> (From update of attachment 112672 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=112672&action=review
> 
> > Source/WebCore/css/CSSStyleSelector.cpp:2139
> > +    return StringHasher::hashMemory(declarations, sizeof(MatchedStyleDeclaration) * size);
> 
> Is there a possibility that this will hash uninitialized memory between the fields of the MatchedStyleDeclaration structure or between elements of the array?

There shouldn't be uninitialize memory between the fields of the MatchedStyleDeclaration struct due to the specific ordering and types used. There won't be any space between the elements of the array as that is forbidden by the standard.

However there might be padding at the end of the struct and I'm unsure if that is guaranteed to be zero-initialized here. If not, it seems to me that the only way to use hashMemory over structs is to explicitly zero-initialize their memory beforehand.

[Antti Koivisto]

Created attachment 112865 [details]
zero initialize the matched declaration struct

The other option would be to use vector traits. However those seemed to be geared towards optimization and being more explicit seemed better.

[Antti Koivisto]

Reopening.

[Gustavo Noronha (kov)]

Comment on attachment 112865 [details]
zero initialize the matched declaration struct

Attachment 112865 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10241057

[Antti Koivisto]

Added explicit zero-initialization, http://trac.webkit.org/changeset/98844