70937 2011-10-26 09:43:42 -0700 Increase StringImpl Flag Bits for 8 bit Strings 2011-10-27 17:53:02 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 66161 1 msaboff msaboff abarth japhet ossy pfeldman rniwa sam webkit.review.bot yurys zherczeg oldest_to_newest 490938 0 msaboff 2011-10-26 09:43:42 -0700 This change is in preparation for adding 8-bit string support. Two flag bits are needed in StringImpl for 8 bit string support. 490951 1 112551 msaboff 2011-10-26 09:54:44 -0700 Created attachment 112551 Proposed patch 490967 2 112551 darin 2011-10-26 10:06:06 -0700 Comment on attachment 112551 Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=112551&action=review > Source/JavaScriptCore/ChangeLog:9 > + Increased the number of bits used for flags in StringImpl > + from 6 to 8 bits. Updated hash methods accordingly. > + Changed hash value masking from the low bits to the high > + bits. This should say why. “This frees up 2 bits that we will be using for 8-bit string support.” 490971 3 msaboff 2011-10-26 10:12:36 -0700 Committed r98495: <http://trac.webkit.org/changeset/98495> 491343 4 rniwa 2011-10-26 21:32:32 -0700 It appears that the following tests started failing after this patch: http/tests/inspector/search/search-in-resources.html inspector/storage-panel-dom-storage.html It's totally unclear why this patch can possibly cause these tests to fail. 491344 5 rniwa 2011-10-26 21:32:57 -0700 http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r98530%20(34228)/results.html FYI, I've manually bisected on my SL machine using Apple's Mac port. 491346 6 rniwa 2011-10-26 21:36:21 -0700 I'm sorry to tell you this 11 hours after your patch was landed but svn server was down for the whole day, bots were unstable, etc... and I could not figure it out earlier :( 491410 7 ossy 2011-10-27 01:45:10 -0700 I can confirm that it broke fast/dom/prototype-inheritance-2.html and inspector/storage-panel-dom-storage.html on the Qt platform too. (with manual bisecting because of svn break) 491421 8 ossy 2011-10-27 02:18:34 -0700 (In reply to comment #7) > I can confirm that it broke fast/dom/prototype-inheritance-2.html and inspector/storage-panel-dom-storage.html on the Qt platform too. (with manual bisecting because of svn break) Only fast/dom/prototype-inheritance-2.html, other test failed because of https://bugs.webkit.org/show_bug.cgi?id=70985 491482 9 zherczeg 2011-10-27 05:35:05 -0700 My impression is that the changed hash codes changed the order of items in some tables. Probably these tests need a rebaseline. 491735 10 sam 2011-10-27 11:08:55 -0700 (In reply to comment #9) > My impression is that the changed hash codes changed the order of items in some tables. Probably these tests need a rebaseline. Indeed. 491773 11 ossy 2011-10-27 11:42:54 -0700 (In reply to comment #10) > (In reply to comment #9) > > My impression is that the changed hash codes changed the order of items in some tables. Probably these tests need a rebaseline. > > Indeed. http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20%28Tests%29/r98530%20%2834228%29/fast/dom/prototype-inheritance-2-pretty-diff.html Why is it order problem? 491874 12 sam 2011-10-27 13:29:45 -0700 (In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #9) > > > My impression is that the changed hash codes changed the order of items in some tables. Probably these tests need a rebaseline. > > > > Indeed. > > http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20%28Tests%29/r98530%20%2834228%29/fast/dom/prototype-inheritance-2-pretty-diff.html > > Why is it order problem? I believe that is just how that test works. Its results are dependent on what happened earlier in the test. 492149 13 msaboff 2011-10-27 17:53:02 -0700 (In reply to comment #12) > (In reply to comment #11) > > (In reply to comment #10) > > > (In reply to comment #9) > > > > My impression is that the changed hash codes changed the order of items in some tables. Probably these tests need a rebaseline. > > > > > > Indeed. > > > > http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20%28Tests%29/r98530%20%2834228%29/fast/dom/prototype-inheritance-2-pretty-diff.html > > > > Why is it order problem? > > I believe that is just how that test works. Its results are dependent on what happened earlier in the test. Patch for review with baseline changes to tests in https://bugs.webkit.org/show_bug.cgi?id=71058 (https://bugs.webkit.org/attachment.cgi?id=112792&action=review). 112551 2011-10-26 09:54:44 -0700 2011-10-26 10:06:06 -0700 Proposed patch 70937.patch text/plain 5698 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gOTg0MTIpCisrKyBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjAgQEAK KzIwMTEtMTAtMjYgIE1pY2hhZWwgU2Fib2ZmICA8bXNhYm9mZkBhcHBsZS5jb20+CisKKyAgICAg ICAgSW5jcmVhc2UgU3RyaW5nSW1wbCBGbGFnIEJpdHMgZm9yIDggYml0IFN0cmluZ3MKKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTcwOTM3CisKKyAgICAg ICAgSW5jcmVhc2VkIHRoZSBudW1iZXIgb2YgYml0cyB1c2VkIGZvciBmbGFncyBpbiBTdHJpbmdJ bXBsCisgICAgICAgIGZyb20gNiB0byA4IGJpdHMuICBVcGRhdGVkIGhhc2ggbWV0aG9kcyBhY2Nv cmRpbmdseS4KKyAgICAgICAgQ2hhbmdlZCBoYXNoIHZhbHVlIG1hc2tpbmcgZnJvbSB0aGUgbG93 IGJpdHMgdG8gdGhlIGhpZ2gKKyAgICAgICAgYml0cy4KKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGNyZWF0ZV9oYXNoX3RhYmxlOgorICAgICAgICAq IHd0Zi9TdHJpbmdIYXNoZXIuaDoKKyAgICAgICAgKFdURjo6U3RyaW5nSGFzaGVyOjpoYXNoKToK KyAgICAgICAgKiB3dGYvdGV4dC9TdHJpbmdJbXBsLmg6CisKIDIwMTEtMTAtMjUgIEdhdmluIEJh cnJhY2xvdWdoICA8YmFycmFjbG91Z2hAYXBwbGUuY29tPgogCiAgICAgICAgIFNlcGFyYXRlIG91 dCBmdW5jdGlvbiBsaW5raW5nICYgZXhjZXB0aW9uIGNoZWNrIGRhdGEgc3RydWN0dXJlcy4KSW5k ZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9jcmVhdGVfaGFzaF90YWJsZQo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvY3JlYXRlX2hhc2hfdGFibGUJKHJldmlzaW9uIDk4Mjg1 KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2NyZWF0ZV9oYXNoX3RhYmxlCSh3b3JraW5nIGNv cHkpCkBAIC0yMjEsMTQgKzIyMSwxNCBAQCBzdWIgaGFzaFZhbHVlKCQpIHsKICAgJGhhc2ggPSAk aGFzaCUgJEVYUDJfMzI7CiAgICRoYXNoIF49IChsZWZ0U2hpZnQoJGhhc2gsIDEwKSUgJEVYUDJf MzIpOwogCi0gICMgU2F2ZSA2IGJpdHMgZm9yIFN0cmluZ0ltcGwgdG8gdXNlIGFzIGZsYWdzLgot ICAkaGFzaCA9ICgkaGFzaCA+PiA2KTsKKyAgIyBTYXZlIDggYml0cyBmb3IgU3RyaW5nSW1wbCB0 byB1c2UgYXMgZmxhZ3MuCisgICRoYXNoICY9IDB4ZmZmZmZmOwogCiAgICMgVGhpcyBhdm9pZHMg ZXZlciByZXR1cm5pbmcgYSBoYXNoIGNvZGUgb2YgMCwgc2luY2UgdGhhdCBpcyB1c2VkIHRvCiAg ICMgc2lnbmFsICJoYXNoIG5vdCBjb21wdXRlZCB5ZXQiLiBTZXR0aW5nIHRoZSBoaWdoIGJpdCBt YWludGFpbnMKICAgIyByZWFzb25hYmxlIGZpZGVsaXR5IHRvIGEgaGFzaCBjb2RlIG9mIDAgYmVj YXVzZSBpdCBpcyBsaWtlbHkgdG8geWllbGQKICAgIyBleGFjdGx5IDAgd2hlbiBoYXNoIGxvb2t1 cCBtYXNrcyBvdXQgdGhlIGhpZ2ggYml0cy4KLSAgJGhhc2ggPSAoMHg4MDAwMDAwMCA+PiA2KSBp ZiAoJGhhc2ggPT0gMCk7CisgICRoYXNoID0gKDB4ODAwMDAwMDAgPj4gOCkgaWYgKCRoYXNoID09 IDApOwogCiAgIHJldHVybiAkaGFzaDsKIH0KSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS93 dGYvU3RyaW5nSGFzaGVyLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL3d0 Zi9TdHJpbmdIYXNoZXIuaAkocmV2aXNpb24gOTgyODUpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENv cmUvd3RmL1N0cmluZ0hhc2hlci5oCSh3b3JraW5nIGNvcHkpCkBAIC0zNiw3ICszNiw3IEBAIHN0 YXRpYyBjb25zdCB1bnNpZ25lZCBzdHJpbmdIYXNoaW5nU3RhcnQKIC8vIEphdmFTY3JpcHRDb3Jl IGFuZCB0aGUgQ29kZUdlbmVyYXRvckpTLnBtIHNjcmlwdCBpbiBXZWJDb3JlLgogY2xhc3MgU3Ry aW5nSGFzaGVyIHsKIHB1YmxpYzoKLSAgICBzdGF0aWMgY29uc3QgdW5zaWduZWQgZmxhZ0NvdW50 ID0gNjsgLy8gU2F2ZSA2IGJpdHMgZm9yIFN0cmluZ0ltcGwgdG8gdXNlIGFzIGZsYWdzLgorICAg IHN0YXRpYyBjb25zdCB1bnNpZ25lZCBmbGFnQ291bnQgPSA4OyAvLyBTYXZlIDggYml0cyBmb3Ig U3RyaW5nSW1wbCB0byB1c2UgYXMgZmxhZ3MuCiAKICAgICBpbmxpbmUgU3RyaW5nSGFzaGVyKCkK ICAgICAgICAgOiBtX2hhc2goc3RyaW5nSGFzaGluZ1N0YXJ0VmFsdWUpCkBAIC04MSw5ICs4MSw5 IEBAIHB1YmxpYzoKICAgICAgICAgcmVzdWx0ICs9IHJlc3VsdCA+PiAxNTsKICAgICAgICAgcmVz dWx0IF49IHJlc3VsdCA8PCAxMDsKIAotICAgICAgICAvLyBSZXNlcnZpbmcgdGhlIGhpZ2ggYml0 cyBmb3IgZmxhZ3MgcHJlc2VydmVzIG1vc3Qgb2YgdGhlIGhhc2gncyB2YWx1ZSwKLSAgICAgICAg Ly8gc2luY2UgaGFzaCBsb29rdXAgdHlwaWNhbGx5IG1hc2tzIG91dCB0aGUgaGlnaCBiaXRzIGFu eXdheS4KLSAgICAgICAgcmVzdWx0ID4+PSBmbGFnQ291bnQ7CisgICAgICAgIC8vIFJlc2Vydmlu ZyBzcGFjZSBmcm9tIHRoZSBoaWdoIGJpdHMgZm9yIGZsYWdzIHByZXNlcnZlcyBtb3N0IG9mIHRo ZSBoYXNoJ3MKKyAgICAgICAgLy8gdmFsdWUsIHNpbmNlIGhhc2ggbG9va3VwIHR5cGljYWxseSBt YXNrcyBvdXQgdGhlIGhpZ2ggYml0cyBhbnl3YXkuCisgICAgICAgIHJlc3VsdCAmPSAoMXUgPDwg KHNpemVvZihyZXN1bHQpICogOCAtIGZsYWdDb3VudCkpIC0gMTsKIAogICAgICAgICAvLyBUaGlz IGF2b2lkcyBldmVyIHJldHVybmluZyBhIGhhc2ggY29kZSBvZiAwLCBzaW5jZSB0aGF0IGlzIHVz ZWQgdG8KICAgICAgICAgLy8gc2lnbmFsICJoYXNoIG5vdCBjb21wdXRlZCB5ZXQiLiBTZXR0aW5n IHRoZSBoaWdoIGJpdCBtYWludGFpbnMKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYv dGV4dC9TdHJpbmdJbXBsLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL3d0 Zi90ZXh0L1N0cmluZ0ltcGwuaAkocmV2aXNpb24gOTgyODUpCisrKyBTb3VyY2UvSmF2YVNjcmlw dENvcmUvd3RmL3RleHQvU3RyaW5nSW1wbC5oCSh3b3JraW5nIGNvcHkpCkBAIC00MDEsOCArNDAx LDggQEAgcHJpdmF0ZToKICAgICBzdGF0aWMgY29uc3QgdW5zaWduZWQgc19yZWZDb3VudEZsYWdJ c1N0YXRpY1N0cmluZyA9IDB4MTsKICAgICBzdGF0aWMgY29uc3QgdW5zaWduZWQgc19yZWZDb3Vu dEluY3JlbWVudCA9IDB4MjsgLy8gVGhpcyBhbGxvd3MgdXMgdG8gcmVmIC8gZGVyZWYgd2l0aG91 dCBkaXN0dXJiaW5nIHRoZSBzdGF0aWMgc3RyaW5nIGZsYWcuCiAKLSAgICAvLyBUaGUgYm90dG9t IDYgYml0cyBpbiB0aGUgaGFzaCBhcmUgZmxhZ3MuCi0gICAgc3RhdGljIGNvbnN0IHVuc2lnbmVk IHNfZmxhZ0NvdW50ID0gNjsKKyAgICAvLyBUaGUgYm90dG9tIDggYml0cyBpbiB0aGUgaGFzaCBh cmUgZmxhZ3MuCisgICAgc3RhdGljIGNvbnN0IHVuc2lnbmVkIHNfZmxhZ0NvdW50ID0gODsKICAg ICBzdGF0aWMgY29uc3QgdW5zaWduZWQgc19mbGFnTWFzayA9ICgxdSA8PCBzX2ZsYWdDb3VudCkg LSAxOwogICAgIENPTVBJTEVfQVNTRVJUKHNfZmxhZ0NvdW50ID09IFN0cmluZ0hhc2hlcjo6Zmxh Z0NvdW50LCBTdHJpbmdIYXNoZXJfcmVzZXJ2ZXNfZW5vdWdoX2JpdHNfZm9yX1N0cmluZ0ltcGxf ZmxhZ3MpOwogCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g U291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCShyZXZpc2lvbiA5ODQxMikKKysrIFNvdXJjZS9XZWJD b3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDExLTEwLTI2 ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAgICAgIEluY3JlYXNl IFN0cmluZ0ltcGwgRmxhZyBCaXRzIGZvciA4IGJpdCBTdHJpbmdzCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03MDkzNworCisgICAgICAgIEluY3JlYXNl ZCB0aGUgbnVtYmVyIG9mIGJpdHMgdXNlZCBmb3IgZmxhZ3MgaW4gU3RyaW5nSW1wbAorICAgICAg ICBmcm9tIDYgdG8gOCBiaXRzLiAgVXBkYXRlZCBoYXNoIG1ldGhvZCBhY2NvcmRpbmdseS4KKyAg ICAgICAgQ2hhbmdlZCBoYXNoIHZhbHVlIG1hc2tpbmcgZnJvbSB0aGUgbG93IGJpdHMgdG8gdGhl IGhpZ2gKKyAgICAgICAgYml0cy4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICAqIGJpbmRpbmdzL3NjcmlwdHMvQ29kZUdlbmVyYXRvckpTLnBtOgorICAg ICAgICAoR2VuZXJhdGVIYXNoVmFsdWUpOgorCiAyMDExLTEwLTI1ICBBZGFtIEJhcnRoICA8YWJh cnRoQHdlYmtpdC5vcmc+CiAKICAgICAgICAgRXZlbnRUYXJnZXRGYWN0b3J5LmluIGlzIG5vdCBz b3J0ZWQKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3NjcmlwdHMvQ29kZUdlbmVyYXRv ckpTLnBtCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3NjcmlwdHMvQ29k ZUdlbmVyYXRvckpTLnBtCShyZXZpc2lvbiA5ODI4NSkKKysrIFNvdXJjZS9XZWJDb3JlL2JpbmRp bmdzL3NjcmlwdHMvQ29kZUdlbmVyYXRvckpTLnBtCSh3b3JraW5nIGNvcHkpCkBAIC0zMDg1LDE0 ICszMDg1LDE0IEBAIHN1YiBHZW5lcmF0ZUhhc2hWYWx1ZQogICAgICRoYXNoID0gJGhhc2glICRF WFAyXzMyOwogICAgICRoYXNoIF49IChsZWZ0U2hpZnQoJGhhc2gsIDEwKSUgJEVYUDJfMzIpOwog ICAgIAotICAgICMgU2F2ZSA2IGJpdHMgZm9yIFN0cmluZ0ltcGwgdG8gdXNlIGFzIGZsYWdzLgot ICAgICRoYXNoID0gKCRoYXNoID4+IDYpOworICAgICMgU2F2ZSA4IGJpdHMgZm9yIFN0cmluZ0lt cGwgdG8gdXNlIGFzIGZsYWdzLgorICAgICRoYXNoICY9IDB4ZmZmZmZmOwogICAgIAogICAgICMg VGhpcyBhdm9pZHMgZXZlciByZXR1cm5pbmcgYSBoYXNoIGNvZGUgb2YgMCwgc2luY2UgdGhhdCBp cyB1c2VkIHRvCiAgICAgIyBzaWduYWwgImhhc2ggbm90IGNvbXB1dGVkIHlldCIuIFNldHRpbmcg dGhlIGhpZ2ggYml0IG1haW50YWlucwogICAgICMgcmVhc29uYWJsZSBmaWRlbGl0eSB0byBhIGhh c2ggY29kZSBvZiAwIGJlY2F1c2UgaXQgaXMgbGlrZWx5IHRvIHlpZWxkCiAgICAgIyBleGFjdGx5 IDAgd2hlbiBoYXNoIGxvb2t1cCBtYXNrcyBvdXQgdGhlIGhpZ2ggYml0cy4KLSAgICAkaGFzaCA9 ICgweDgwMDAwMDAwID4+IDYpIGlmICgkaGFzaCA9PSAwKTsKKyAgICAkaGFzaCA9ICgweDgwMDAw MDAwID4+IDgpIGlmICgkaGFzaCA9PSAwKTsKICAgICAKICAgICByZXR1cm4gJGhhc2g7CiB9Cg==