46516 2010-09-24 14:41:01 -0700 Use WTF::StringHasher for hashing MappedAttributeKey 2010-10-09 08:32:23 -0700 1 1 1 Unclassified WebKit Web Template Framework 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 paroga paroga abarth commit-queue oldest_to_newest 284713 0 paroga 2010-09-24 14:41:01 -0700 see patch 284716 1 68753 paroga 2010-09-24 14:43:02 -0700 Created attachment 68753 Patch 285297 2 68753 abarth 2010-09-26 21:39:55 -0700 Comment on attachment 68753 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=68753&action=review Do we want to run any performance tests to make sure we're not regressing here? > WebCore/dom/StyledElement.cpp:-456 > - // This avoids ever returning a hash code of 0, since that is used to > - // signal "hash not computed yet", using a value that is likely to be > - // effectively the same as 0 when the low bits are masked > - if (hash == 0) > - hash = 0x80000000; Does StringHasher respect this? 285346 3 paroga 2010-09-26 23:53:52 -0700 (In reply to comment #2) > (From update of attachment 68753 [details]) > Do we want to run any performance tests to make sure we're not regressing here? StringHasher has only inline methods. The only real difference is the "pendingCharacter" (http://trac.webkit.org/browser/trunk/JavaScriptCore/wtf/StringHashFunctions.h?rev=68369#L66), but a intelligent compiler should be able optimize it away. > > WebCore/dom/StyledElement.cpp:-456 > > - // This avoids ever returning a hash code of 0, since that is used to > > - // signal "hash not computed yet", using a value that is likely to be > > - // effectively the same as 0 when the low bits are masked > > - if (hash == 0) > > - hash = 0x80000000; > > Does StringHasher respect this? StringHasher uses 0x40000000 instead (http://trac.webkit.org/browser/trunk/JavaScriptCore/wtf/StringHashFunctions.h?rev=68369#L85), but this shoudn't make a difference, because only "not null" is relevant. 285359 4 68885 paroga 2010-09-27 00:36:33 -0700 Created attachment 68885 Patch Fixed copy/paste failure: > COMPILE_ASSERT(sizeof(key.name) == 4 || sizeof(key.name) == 8, key_name_size); > COMPILE_ASSERT(sizeof(key.value) == 4 || sizeof(key.value) == 8, key_name_size); 286146 5 68753 eric 2010-09-28 03:19:24 -0700 Comment on attachment 68753 Patch Cleared Adam Barth's review+ from obsolete attachment 68753 so that this bug does not appear in http://webkit.org/pending-commit. 292082 6 paroga 2010-10-09 02:55:59 -0700 (In reply to comment #2) > (From update of attachment 68753 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=68753&action=review > > Do we want to run any performance tests to make sure we're not regressing here? I now run WebCore/benchmarks/parser/html-parser.html to check the difference: BEFORE PATCH: avg 4039.95 stdev 28.554290395665586 avg 4042.90 stdev 26.95904300972125 avg 4054.45 stdev 35.382870149268555 avg 4060.05 stdev 41.9147647017134 AFTER PATCH: avg 4044.30 stdev 34.20833231831099 avg 4055.15 stdev 28.320090042229737 avg 4055.40 stdev 29.014479144041164 avg 4062.60 stdev 39.329886854655456 Is the difference within the measuring tolerance? 292108 7 abarth 2010-10-09 08:12:16 -0700 > Is the difference within the measuring tolerance? Yep. Thanks. 292120 8 68885 commit-queue 2010-10-09 08:32:18 -0700 Comment on attachment 68885 Patch Clearing flags on attachment: 68885 Committed r69449: <http://trac.webkit.org/changeset/69449> 292121 9 commit-queue 2010-10-09 08:32:23 -0700 All reviewed patches have been landed. Closing bug. 68753 2010-09-24 14:43:02 -0700 2010-09-28 03:19:24 -0700 Patch bug-46516.patch text/plain 3180 paroga ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg ZTQwZDE2Ny4uMjY1NmViOSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxMyBAQAorMjAxMC0wOS0yNSAgUGF0cmljayBHYW5z dGVyZXIgIDxwYXJvZ2FAd2Via2l0Lm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBVc2UgV1RGOjpTdHJpbmdIYXNoZXIgZm9yIGhhc2hpbmcgTWFw cGVkQXR0cmlidXRlS2V5CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD00NjUxNgorCisgICAgICAgICogZG9tL1N0eWxlZEVsZW1lbnQuY3BwOgorICAgICAg ICAoV2ViQ29yZTo6TWFwcGVkQXR0cmlidXRlSGFzaDo6aGFzaCk6CisKIDIwMTAtMDktMjQgIERh dmlkIEh5YXR0ICA8aHlhdHRAYXBwbGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IFNhbSBX ZWluaWcuCmRpZmYgLS1naXQgYS9XZWJDb3JlL2RvbS9TdHlsZWRFbGVtZW50LmNwcCBiL1dlYkNv cmUvZG9tL1N0eWxlZEVsZW1lbnQuY3BwCmluZGV4IGExNThlMmUuLjczNGY2MzcgMTAwNjQ0Ci0t LSBhL1dlYkNvcmUvZG9tL1N0eWxlZEVsZW1lbnQuY3BwCisrKyBiL1dlYkNvcmUvZG9tL1N0eWxl ZEVsZW1lbnQuY3BwCkBAIC00MDAsNjIgKzQwMCwyNSBAQCB2b2lkIFN0eWxlZEVsZW1lbnQ6OmNy ZWF0ZU1hcHBlZERlY2woQXR0cmlidXRlKiBhdHRyKQogICAgIGRlY2wtPnNldFN0cmljdFBhcnNp bmcoZmFsc2UpOyAvLyBNYXBwZWQgYXR0cmlidXRlcyBhcmUganVzdCBhbHdheXMgcXVpcmt5Lgog fQogCi0vLyBQYXVsIEhzaWVoJ3MgU3VwZXJGYXN0SGFzaAotLy8gaHR0cDovL3d3dy5hemlsbGlv bm1vbmtleXMuY29tL3FlZC9oYXNoLmh0bWwKIHVuc2lnbmVkIE1hcHBlZEF0dHJpYnV0ZUhhc2g6 Omhhc2goY29uc3QgTWFwcGVkQXR0cmlidXRlS2V5JiBrZXkpCiB7Ci0gICAgdWludDMyX3QgaGFz aCA9IFdURjo6c3RyaW5nSGFzaGluZ1N0YXJ0VmFsdWU7Ci0gICAgdWludDMyX3QgdG1wOwotCi0g ICAgY29uc3QgdWludDE2X3QqIHA7Ci0KLSAgICBwID0gcmVpbnRlcnByZXRfY2FzdDxjb25zdCB1 aW50MTZfdCo+KCZrZXkubmFtZSk7Ci0gICAgaGFzaCArPSBwWzBdOwotICAgIHRtcCA9IChwWzFd IDw8IDExKSBeIGhhc2g7Ci0gICAgaGFzaCA9IChoYXNoIDw8IDE2KSBeIHRtcDsKLSAgICBoYXNo ICs9IGhhc2ggPj4gMTE7Ci0gICAgQVNTRVJUKHNpemVvZihrZXkubmFtZSkgPT0gNCB8fCBzaXpl b2Yoa2V5Lm5hbWUpID09IDgpOwotICAgIGlmIChzaXplb2Yoa2V5Lm5hbWUpID09IDgpIHsKLSAg ICAgICAgcCArPSAyOwotICAgICAgICBoYXNoICs9IHBbMF07Ci0gICAgICAgIHRtcCA9IChwWzFd IDw8IDExKSBeIGhhc2g7Ci0gICAgICAgIGhhc2ggPSAoaGFzaCA8PCAxNikgXiB0bXA7Ci0gICAg ICAgIGhhc2ggKz0gaGFzaCA+PiAxMTsKLSAgICB9CisgICAgQ09NUElMRV9BU1NFUlQoc2l6ZW9m KGtleS5uYW1lKSA9PSA0IHx8IHNpemVvZihrZXkubmFtZSkgPT0gOCwga2V5X25hbWVfc2l6ZSk7 CisgICAgQ09NUElMRV9BU1NFUlQoc2l6ZW9mKGtleS52YWx1ZSkgPT0gNCB8fCBzaXplb2Yoa2V5 LnZhbHVlKSA9PSA4LCBrZXlfbmFtZV9zaXplKTsKIAotICAgIHAgPSByZWludGVycHJldF9jYXN0 PGNvbnN0IHVpbnQxNl90Kj4oJmtleS52YWx1ZSk7Ci0gICAgaGFzaCArPSBwWzBdOwotICAgIHRt cCA9IChwWzFdIDw8IDExKSBeIGhhc2g7Ci0gICAgaGFzaCA9IChoYXNoIDw8IDE2KSBeIHRtcDsK LSAgICBoYXNoICs9IGhhc2ggPj4gMTE7Ci0gICAgQVNTRVJUKHNpemVvZihrZXkudmFsdWUpID09 IDQgfHwgc2l6ZW9mKGtleS52YWx1ZSkgPT0gOCk7Ci0gICAgaWYgKHNpemVvZihrZXkudmFsdWUp ID09IDgpIHsKLSAgICAgICAgcCArPSAyOwotICAgICAgICBoYXNoICs9IHBbMF07Ci0gICAgICAg IHRtcCA9IChwWzFdIDw8IDExKSBeIGhhc2g7Ci0gICAgICAgIGhhc2ggPSAoaGFzaCA8PCAxNikg XiB0bXA7Ci0gICAgICAgIGhhc2ggKz0gaGFzaCA+PiAxMTsKLSAgICB9CisgICAgV1RGOjpTdHJp bmdIYXNoZXIgaGFzaGVyOworICAgIGNvbnN0IFVDaGFyKiBkYXRhOworCisgICAgZGF0YSA9IHJl aW50ZXJwcmV0X2Nhc3Q8Y29uc3QgVUNoYXIqPigma2V5Lm5hbWUpOworICAgIGhhc2hlci5hZGRD aGFyYWN0ZXJzKGRhdGFbMF0sIGRhdGFbMV0pOworICAgIGlmIChzaXplb2Yoa2V5Lm5hbWUpID09 IDgpCisgICAgICAgIGhhc2hlci5hZGRDaGFyYWN0ZXJzKGRhdGFbMl0sIGRhdGFbM10pOworCisg ICAgZGF0YSA9IHJlaW50ZXJwcmV0X2Nhc3Q8Y29uc3QgVUNoYXIqPigma2V5LnZhbHVlKTsKKyAg ICBoYXNoZXIuYWRkQ2hhcmFjdGVycyhkYXRhWzBdLCBkYXRhWzFdKTsKKyAgICBpZiAoc2l6ZW9m KGtleS52YWx1ZSkgPT0gOCkKKyAgICAgICAgaGFzaGVyLmFkZENoYXJhY3RlcnMoZGF0YVsyXSwg ZGF0YVszXSk7CiAKLSAgICAvLyBIYW5kbGUgZW5kIGNhc2UKLSAgICBoYXNoICs9IGtleS50eXBl OwotICAgIGhhc2ggXj0gaGFzaCA8PCAxMTsKLSAgICBoYXNoICs9IGhhc2ggPj4gMTc7Ci0KLSAg ICAvLyBGb3JjZSAiYXZhbGFuY2hpbmciIG9mIGZpbmFsIDEyNyBiaXRzCi0gICAgaGFzaCBePSBo YXNoIDw8IDM7Ci0gICAgaGFzaCArPSBoYXNoID4+IDU7Ci0gICAgaGFzaCBePSBoYXNoIDw8IDI7 Ci0gICAgaGFzaCArPSBoYXNoID4+IDE1OwotICAgIGhhc2ggXj0gaGFzaCA8PCAxMDsKLQotICAg IC8vIFRoaXMgYXZvaWRzIGV2ZXIgcmV0dXJuaW5nIGEgaGFzaCBjb2RlIG9mIDAsIHNpbmNlIHRo YXQgaXMgdXNlZCB0bwotICAgIC8vIHNpZ25hbCAiaGFzaCBub3QgY29tcHV0ZWQgeWV0IiwgdXNp bmcgYSB2YWx1ZSB0aGF0IGlzIGxpa2VseSB0byBiZQotICAgIC8vIGVmZmVjdGl2ZWx5IHRoZSBz YW1lIGFzIDAgd2hlbiB0aGUgbG93IGJpdHMgYXJlIG1hc2tlZAotICAgIGlmIChoYXNoID09IDAp Ci0gICAgICAgIGhhc2ggPSAweDgwMDAwMDAwOwotCi0gICAgcmV0dXJuIGhhc2g7CisgICAgcmV0 dXJuIGhhc2hlci5oYXNoKCk7CiB9CiAKIHZvaWQgU3R5bGVkRWxlbWVudDo6Y29weU5vbkF0dHJp YnV0ZVByb3BlcnRpZXMoY29uc3QgRWxlbWVudCAqc291cmNlRWxlbWVudCkK 68885 2010-09-27 00:36:33 -0700 2010-10-09 08:32:18 -0700 Patch bug-46516b.patch text/plain 3177 paroga ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg ZTQwZDE2Ny4uMjY1NmViOSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxMyBAQAorMjAxMC0wOS0yNSAgUGF0cmljayBHYW5z dGVyZXIgIDxwYXJvZ2FAd2Via2l0Lm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBBZGFtIEJh cnRoLgorCisgICAgICAgIFVzZSBXVEY6OlN0cmluZ0hhc2hlciBmb3IgaGFzaGluZyBNYXBwZWRB dHRyaWJ1dGVLZXkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTQ2NTE2CisKKyAgICAgICAgKiBkb20vU3R5bGVkRWxlbWVudC5jcHA6CisgICAgICAgIChX ZWJDb3JlOjpNYXBwZWRBdHRyaWJ1dGVIYXNoOjpoYXNoKToKKwogMjAxMC0wOS0yNCAgRGF2aWQg SHlhdHQgIDxoeWF0dEBhcHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgU2FtIFdlaW5p Zy4KZGlmZiAtLWdpdCBhL1dlYkNvcmUvZG9tL1N0eWxlZEVsZW1lbnQuY3BwIGIvV2ViQ29yZS9k b20vU3R5bGVkRWxlbWVudC5jcHAKaW5kZXggYTE1OGUyZS4uNzM0ZjYzNyAxMDA2NDQKLS0tIGEv V2ViQ29yZS9kb20vU3R5bGVkRWxlbWVudC5jcHAKKysrIGIvV2ViQ29yZS9kb20vU3R5bGVkRWxl bWVudC5jcHAKQEAgLTQwMCw2MiArNDAwLDI1IEBAIHZvaWQgU3R5bGVkRWxlbWVudDo6Y3JlYXRl TWFwcGVkRGVjbChBdHRyaWJ1dGUqIGF0dHIpCiAgICAgZGVjbC0+c2V0U3RyaWN0UGFyc2luZyhm YWxzZSk7IC8vIE1hcHBlZCBhdHRyaWJ1dGVzIGFyZSBqdXN0IGFsd2F5cyBxdWlya3kuCiB9CiAK LS8vIFBhdWwgSHNpZWgncyBTdXBlckZhc3RIYXNoCi0vLyBodHRwOi8vd3d3LmF6aWxsaW9ubW9u a2V5cy5jb20vcWVkL2hhc2guaHRtbAogdW5zaWduZWQgTWFwcGVkQXR0cmlidXRlSGFzaDo6aGFz aChjb25zdCBNYXBwZWRBdHRyaWJ1dGVLZXkmIGtleSkKIHsKLSAgICB1aW50MzJfdCBoYXNoID0g V1RGOjpzdHJpbmdIYXNoaW5nU3RhcnRWYWx1ZTsKLSAgICB1aW50MzJfdCB0bXA7Ci0KLSAgICBj b25zdCB1aW50MTZfdCogcDsKLQotICAgIHAgPSByZWludGVycHJldF9jYXN0PGNvbnN0IHVpbnQx Nl90Kj4oJmtleS5uYW1lKTsKLSAgICBoYXNoICs9IHBbMF07Ci0gICAgdG1wID0gKHBbMV0gPDwg MTEpIF4gaGFzaDsKLSAgICBoYXNoID0gKGhhc2ggPDwgMTYpIF4gdG1wOwotICAgIGhhc2ggKz0g aGFzaCA+PiAxMTsKLSAgICBBU1NFUlQoc2l6ZW9mKGtleS5uYW1lKSA9PSA0IHx8IHNpemVvZihr ZXkubmFtZSkgPT0gOCk7Ci0gICAgaWYgKHNpemVvZihrZXkubmFtZSkgPT0gOCkgewotICAgICAg ICBwICs9IDI7Ci0gICAgICAgIGhhc2ggKz0gcFswXTsKLSAgICAgICAgdG1wID0gKHBbMV0gPDwg MTEpIF4gaGFzaDsKLSAgICAgICAgaGFzaCA9IChoYXNoIDw8IDE2KSBeIHRtcDsKLSAgICAgICAg aGFzaCArPSBoYXNoID4+IDExOwotICAgIH0KKyAgICBDT01QSUxFX0FTU0VSVChzaXplb2Yoa2V5 Lm5hbWUpID09IDQgfHwgc2l6ZW9mKGtleS5uYW1lKSA9PSA4LCBrZXlfbmFtZV9zaXplKTsKKyAg ICBDT01QSUxFX0FTU0VSVChzaXplb2Yoa2V5LnZhbHVlKSA9PSA0IHx8IHNpemVvZihrZXkudmFs dWUpID09IDgsIGtleV92YWx1ZV9zaXplKTsKIAotICAgIHAgPSByZWludGVycHJldF9jYXN0PGNv bnN0IHVpbnQxNl90Kj4oJmtleS52YWx1ZSk7Ci0gICAgaGFzaCArPSBwWzBdOwotICAgIHRtcCA9 IChwWzFdIDw8IDExKSBeIGhhc2g7Ci0gICAgaGFzaCA9IChoYXNoIDw8IDE2KSBeIHRtcDsKLSAg ICBoYXNoICs9IGhhc2ggPj4gMTE7Ci0gICAgQVNTRVJUKHNpemVvZihrZXkudmFsdWUpID09IDQg fHwgc2l6ZW9mKGtleS52YWx1ZSkgPT0gOCk7Ci0gICAgaWYgKHNpemVvZihrZXkudmFsdWUpID09 IDgpIHsKLSAgICAgICAgcCArPSAyOwotICAgICAgICBoYXNoICs9IHBbMF07Ci0gICAgICAgIHRt cCA9IChwWzFdIDw8IDExKSBeIGhhc2g7Ci0gICAgICAgIGhhc2ggPSAoaGFzaCA8PCAxNikgXiB0 bXA7Ci0gICAgICAgIGhhc2ggKz0gaGFzaCA+PiAxMTsKLSAgICB9CisgICAgV1RGOjpTdHJpbmdI YXNoZXIgaGFzaGVyOworICAgIGNvbnN0IFVDaGFyKiBkYXRhOworCisgICAgZGF0YSA9IHJlaW50 ZXJwcmV0X2Nhc3Q8Y29uc3QgVUNoYXIqPigma2V5Lm5hbWUpOworICAgIGhhc2hlci5hZGRDaGFy YWN0ZXJzKGRhdGFbMF0sIGRhdGFbMV0pOworICAgIGlmIChzaXplb2Yoa2V5Lm5hbWUpID09IDgp CisgICAgICAgIGhhc2hlci5hZGRDaGFyYWN0ZXJzKGRhdGFbMl0sIGRhdGFbM10pOworCisgICAg ZGF0YSA9IHJlaW50ZXJwcmV0X2Nhc3Q8Y29uc3QgVUNoYXIqPigma2V5LnZhbHVlKTsKKyAgICBo YXNoZXIuYWRkQ2hhcmFjdGVycyhkYXRhWzBdLCBkYXRhWzFdKTsKKyAgICBpZiAoc2l6ZW9mKGtl eS52YWx1ZSkgPT0gOCkKKyAgICAgICAgaGFzaGVyLmFkZENoYXJhY3RlcnMoZGF0YVsyXSwgZGF0 YVszXSk7CiAKLSAgICAvLyBIYW5kbGUgZW5kIGNhc2UKLSAgICBoYXNoICs9IGtleS50eXBlOwot ICAgIGhhc2ggXj0gaGFzaCA8PCAxMTsKLSAgICBoYXNoICs9IGhhc2ggPj4gMTc7Ci0KLSAgICAv LyBGb3JjZSAiYXZhbGFuY2hpbmciIG9mIGZpbmFsIDEyNyBiaXRzCi0gICAgaGFzaCBePSBoYXNo IDw8IDM7Ci0gICAgaGFzaCArPSBoYXNoID4+IDU7Ci0gICAgaGFzaCBePSBoYXNoIDw8IDI7Ci0g ICAgaGFzaCArPSBoYXNoID4+IDE1OwotICAgIGhhc2ggXj0gaGFzaCA8PCAxMDsKLQotICAgIC8v IFRoaXMgYXZvaWRzIGV2ZXIgcmV0dXJuaW5nIGEgaGFzaCBjb2RlIG9mIDAsIHNpbmNlIHRoYXQg aXMgdXNlZCB0bwotICAgIC8vIHNpZ25hbCAiaGFzaCBub3QgY29tcHV0ZWQgeWV0IiwgdXNpbmcg YSB2YWx1ZSB0aGF0IGlzIGxpa2VseSB0byBiZQotICAgIC8vIGVmZmVjdGl2ZWx5IHRoZSBzYW1l IGFzIDAgd2hlbiB0aGUgbG93IGJpdHMgYXJlIG1hc2tlZAotICAgIGlmIChoYXNoID09IDApCi0g ICAgICAgIGhhc2ggPSAweDgwMDAwMDAwOwotCi0gICAgcmV0dXJuIGhhc2g7CisgICAgcmV0dXJu IGhhc2hlci5oYXNoKCk7CiB9CiAKIHZvaWQgU3R5bGVkRWxlbWVudDo6Y29weU5vbkF0dHJpYnV0 ZVByb3BlcnRpZXMoY29uc3QgRWxlbWVudCAqc291cmNlRWxlbWVudCkK