71012 2011-10-27 06:15:03 -0700 Use StringHasher to generate the matched declaration cache hash 2011-10-31 04:25:19 -0700 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 koivisto webkit-unassigned darin gustavo macpherson webkit.review.bot xan.lopez oldest_to_newest 491510 0 koivisto 2011-10-27 06:15:03 -0700 It is faster and better than the current custom function. 491514 1 112672 koivisto 2011-10-27 06:20:38 -0700 Created attachment 112672 patch 491515 2 112672 kenneth 2011-10-27 06:22:19 -0700 Comment on attachment 112672 patch r=me 491577 3 koivisto 2011-10-27 08:04:14 -0700 http://trac.webkit.org/changeset/98573 491658 4 112672 darin 2011-10-27 10:05:07 -0700 Comment on attachment 112672 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? 491866 5 koivisto 2011-10-27 13:22:43 -0700 (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. 492458 6 112865 koivisto 2011-10-28 08:00:37 -0700 Created attachment 112865 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. 492459 7 koivisto 2011-10-28 08:01:31 -0700 Reopening. 492468 8 112865 gustavo 2011-10-28 08:31:00 -0700 Comment on attachment 112865 zero initialize the matched declaration struct Attachment 112865 did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/10241057 493244 9 koivisto 2011-10-31 04:25:19 -0700 Added explicit zero-initialization, http://trac.webkit.org/changeset/98844 112672 2011-10-27 06:20:38 -0700 2011-10-27 10:05:07 -0700 patch declaration-hash.patch text/plain 1679 koivisto SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDk4NTU5KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMTAtMjcgIEFudHRpIEtv aXZpc3RvICA8YW50dGlAYXBwbGUuY29tPgorCisgICAgICAgIFVzZSBTdHJpbmdIYXNoZXIgdG8g Z2VuZXJhdGUgdGhlIG1hdGNoZWQgZGVjbGFyYXRpb24gY2FjaGUgaGFzaAorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NzEwMTIKKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBJdCBpcyBmYXN0ZXIgYW5kIGJldHRl ciB0aGFuIHRoZSBjdXJyZW50IGN1c3RvbSBmdW5jdGlvbi4KKworICAgICAgICAqIGNzcy9DU1NT dHlsZVNlbGVjdG9yLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkNTU1N0eWxlU2VsZWN0b3I6OmNv bXB1dGVEZWNsYXJhdGlvbkhhc2gpOgorCiAyMDExLTEwLTI3ICBBbmRyYXMgQmVjc2kgIDxhbmRy YXMuYmVjc2lAbm9raWEuY29tPgogCiAgICAgICAgIEZpeCB0aGUgYnVpbGQgaWYgTk9fTElTVEJP WF9SRU5ERVJJTkcgaXMgZW5hYmxlZApJbmRleDogU291cmNlL1dlYkNvcmUvY3NzL0NTU1N0eWxl U2VsZWN0b3IuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL2Nzcy9DU1NTdHlsZVNl bGVjdG9yLmNwcAkocmV2aXNpb24gOTg1NDIpCisrKyBTb3VyY2UvV2ViQ29yZS9jc3MvQ1NTU3R5 bGVTZWxlY3Rvci5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTIxMzYsMTQgKzIxMzYsNyBAQCB2b2lk IENTU1N0eWxlU2VsZWN0b3I6OmFwcGx5RGVjbGFyYXRpb25zCiAKIHVuc2lnbmVkIENTU1N0eWxl U2VsZWN0b3I6OmNvbXB1dGVEZWNsYXJhdGlvbkhhc2goTWF0Y2hlZFN0eWxlRGVjbGFyYXRpb24q IGRlY2xhcmF0aW9ucywgdW5zaWduZWQgc2l6ZSkKIHsKLSAgICB1bnNpZ25lZCBoYXNoID0gMDsK LSAgICBmb3IgKHVuc2lnbmVkIGkgPSAwOyBpIDwgc2l6ZTsgKytpKSB7Ci0gICAgICAgIHVuc2ln bmVkIHB0ckhhc2ggPSBQdHJIYXNoPENTU011dGFibGVTdHlsZURlY2xhcmF0aW9uKj46Omhhc2go ZGVjbGFyYXRpb25zW2ldLnN0eWxlRGVjbGFyYXRpb24pOwotICAgICAgICBwdHJIYXNoIF49IElu dEhhc2g8dW5zaWduZWQ+OjpoYXNoKGRlY2xhcmF0aW9uc1tpXS5saW5rTWF0Y2hUeXBlKTsKLSAg ICAgICAgLy8gTWFrZSB0aGUgcG9zaXRpb24gbWF0dGVyLgotICAgICAgICBoYXNoIF49IChwdHJI YXNoIDw8IGkpIHwgKHB0ckhhc2ggPj4gKDMyIC0gaSkpOwotICAgIH0KLSAgICByZXR1cm4gaGFz aDsKKyAgICByZXR1cm4gU3RyaW5nSGFzaGVyOjpoYXNoTWVtb3J5KGRlY2xhcmF0aW9ucywgc2l6 ZW9mKE1hdGNoZWRTdHlsZURlY2xhcmF0aW9uKSAqIHNpemUpOwogfQogCiBib29sIG9wZXJhdG9y PT0oY29uc3QgQ1NTU3R5bGVTZWxlY3Rvcjo6TWF0Y2hSZXN1bHQmIGEsIGNvbnN0IENTU1N0eWxl U2VsZWN0b3I6Ok1hdGNoUmVzdWx0JiBiKQo= 112865 2011-10-28 08:00:37 -0700 2011-10-30 11:57:33 -0700 zero initialize the matched declaration struct zero-initialize-matched.patch text/plain 2670 koivisto SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDk4NzI3KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTEtMTAtMjggIEFudHRpIEtv aXZpc3RvICA8YW50dGlAYXBwbGUuY29tPgorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD03MTAxMgorICAgICAgICBVc2UgU3RyaW5nSGFzaGVyIHRvIGdl bmVyYXRlIHRoZSBtYXRjaGVkIGRlY2xhcmF0aW9uIGNhY2hlIGhhc2gKKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBNYWtlIHN1cmUgdGhlIE1hdGNoZWRT dHlsZURlY2xhcmF0aW9ucyBhcmUgZnVsbHkgemVyby1pbml0aWFsaXplZCBhcyB3ZSBjYWxjdWxh dGUgYSBoYXNoIG92ZXIgYSByYXcgbWVtb3J5IGFycmF5LgorCisgICAgICAgICogY3NzL0NTU1N0 eWxlU2VsZWN0b3IuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Q1NTU3R5bGVTZWxlY3Rvcjo6TWF0 Y2hlZFN0eWxlRGVjbGFyYXRpb246Ok1hdGNoZWRTdHlsZURlY2xhcmF0aW9uKToKKyAgICAgICAg KFdlYkNvcmU6OkNTU1N0eWxlU2VsZWN0b3I6OmFkZE1hdGNoZWREZWNsYXJhdGlvbik6CisgICAg ICAgICogY3NzL0NTU1N0eWxlU2VsZWN0b3IuaDoKKwogMjAxMS0xMC0yOCAgUGF2ZWwgRmVsZG1h biAgPHBmZWxkbWFuQGdvb2dsZS5jb20+CiAKICAgICAgICAgUmVzZXQgbGluZSBudW1iZXJzIGZv ciBzY3JpcHRzIGdlbmVyYXRlZCB3aXRoIGRvY3VtZW50LndyaXRlLgpJbmRleDogU291cmNlL1dl YkNvcmUvY3NzL0NTU1N0eWxlU2VsZWN0b3IuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJD b3JlL2Nzcy9DU1NTdHlsZVNlbGVjdG9yLmNwcAkocmV2aXNpb24gOTg3MjQpCisrKyBTb3VyY2Uv V2ViQ29yZS9jc3MvQ1NTU3R5bGVTZWxlY3Rvci5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTU5Myw5 ICs1OTMsMTggQEAgc3RhdGljIHZvaWQgZW5zdXJlRGVmYXVsdFN0eWxlU2hlZXRzRm9yRQogI2Vu ZGlmCiB9CiAKLXZvaWQgQ1NTU3R5bGVTZWxlY3Rvcjo6YWRkTWF0Y2hlZERlY2xhcmF0aW9uKENT U011dGFibGVTdHlsZURlY2xhcmF0aW9uKiBkZWNsLCB1bnNpZ25lZCBsaW5rTWF0Y2hUeXBlKQot ewotICAgIG1fbWF0Y2hlZERlY2xzLmFwcGVuZChNYXRjaGVkU3R5bGVEZWNsYXJhdGlvbihkZWNs LCBsaW5rTWF0Y2hUeXBlKSk7CitDU1NTdHlsZVNlbGVjdG9yOjpNYXRjaGVkU3R5bGVEZWNsYXJh dGlvbjo6TWF0Y2hlZFN0eWxlRGVjbGFyYXRpb24oKSAKK3sgIAorICAgIC8vIE1ha2Ugc3VyZSBh bGwgbWVtb3J5IGlzIHplcm8gaW5pdGlhbGl6ZXMgYXMgd2UgY2FsY3VsYXRlIGhhc2ggb3ZlciB0 aGUgYnl0ZXMgb2YgdGhpcyBvYmplY3QuCisgICAgbWVtc2V0KHRoaXMsIDAsIHNpemVvZigqdGhp cykpOworfQorCit2b2lkIENTU1N0eWxlU2VsZWN0b3I6OmFkZE1hdGNoZWREZWNsYXJhdGlvbihD U1NNdXRhYmxlU3R5bGVEZWNsYXJhdGlvbiogc3R5bGVEZWNsYXJhdGlvbiwgdW5zaWduZWQgbGlu a01hdGNoVHlwZSkKK3sKKyAgICBtX21hdGNoZWREZWNscy5ncm93KG1fbWF0Y2hlZERlY2xzLnNp emUoKSArIDEpOworICAgIE1hdGNoZWRTdHlsZURlY2xhcmF0aW9uJiBuZXdEZWNsYXJhdGlvbiA9 IG1fbWF0Y2hlZERlY2xzLmxhc3QoKTsKKyAgICBuZXdEZWNsYXJhdGlvbi5zdHlsZURlY2xhcmF0 aW9uID0gc3R5bGVEZWNsYXJhdGlvbjsKKyAgICBuZXdEZWNsYXJhdGlvbi5saW5rTWF0Y2hUeXBl ID0gbGlua01hdGNoVHlwZTsKIH0KIAogdm9pZCBDU1NTdHlsZVNlbGVjdG9yOjptYXRjaFJ1bGVz KFJ1bGVTZXQqIHJ1bGVzLCBpbnQmIGZpcnN0UnVsZUluZGV4LCBpbnQmIGxhc3RSdWxlSW5kZXgs IGJvb2wgaW5jbHVkZUVtcHR5UnVsZXMpCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9jc3MvQ1NTU3R5 bGVTZWxlY3Rvci5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL2Nzcy9DU1NTdHlsZVNl bGVjdG9yLmgJKHJldmlzaW9uIDk4NzI0KQorKysgU291cmNlL1dlYkNvcmUvY3NzL0NTU1N0eWxl U2VsZWN0b3IuaAkod29ya2luZyBjb3B5KQpAQCAtMzMwLDcgKzMzMCw3IEBAIHByaXZhdGU6CiAg ICAgdm9pZCBsb2FkUGVuZGluZ0ltYWdlcygpOwogCiAgICAgc3RydWN0IE1hdGNoZWRTdHlsZURl Y2xhcmF0aW9uIHsKLSAgICAgICAgTWF0Y2hlZFN0eWxlRGVjbGFyYXRpb24oQ1NTTXV0YWJsZVN0 eWxlRGVjbGFyYXRpb24qIGRlY2wsIHVuc2lnbmVkIHR5cGUpIDogc3R5bGVEZWNsYXJhdGlvbihk ZWNsKSwgbGlua01hdGNoVHlwZSh0eXBlKSB7IH0KKyAgICAgICAgTWF0Y2hlZFN0eWxlRGVjbGFy YXRpb24oKTsKICAgICAgICAgQ1NTTXV0YWJsZVN0eWxlRGVjbGFyYXRpb24qIHN0eWxlRGVjbGFy YXRpb247CiAgICAgICAgIHVuc2lnbmVkIGxpbmtNYXRjaFR5cGU7CiAgICAgfTsK