28664 2009-08-22 23:33:26 -0700 Array index miss case creates a string every time 2009-08-24 14:38:00 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Enhancement --- 1 darin darin ddkilzer oldest_to_newest 142185 0 darin 2009-08-22 23:33:26 -0700 The object prototype is creating a string and searching for it in the Identifier table every time there's a miss. This is bad on the crypto tests in SunSpider, and in other real-world code too I'm sure. 142186 1 38446 darin 2009-08-22 23:36:59 -0700 Created attachment 38446 patch 142190 2 38446 darin 2009-08-22 23:45:46 -0700 Comment on attachment 38446 patch Maybe it should be called m_mayHaveIntegralNamedProperty because I don't have any code to set it to false if the offending properties are all removed. 142191 3 38446 darin 2009-08-22 23:46:55 -0700 Comment on attachment 38446 patch Or I could reverse its sense and call this m_hasNoIntegralNamedProperties. 142226 4 38446 ddkilzer 2009-08-23 05:55:13 -0700 Comment on attachment 38446 patch >+ if (isUInt32) >+ m_hasIntegralNamedProperty = true; You could save a branch here by always assigning m_hasIntegralNamedProperty: m_hasIntegralNamedProperty = isUInt32; Not sure if that's helpful or not, though. 142242 5 darin 2009-08-23 14:07:30 -0700 (In reply to comment #4) > (From update of attachment 38446 [details]) > >+ if (isUInt32) > >+ m_hasIntegralNamedProperty = true; > > You could save a branch here by always assigning m_hasIntegralNamedProperty: > > m_hasIntegralNamedProperty = isUInt32; > > Not sure if that's helpful or not, though. That would break the case where isUInt32 is false and m_hasIntegralNamedProperty is already true, but I could do: m_hasIntegralNamedProperty |= isUInt32; Or I could skip the entire check if m_hasIntegralNamedProperty is already true. No need to call toStrictUInt32 at all in that case. But none of that seems important. We don't need to optimize adding properties to the object prototypes. We should optimize for clarity. My existing patch seems OK to me, but we could make changes. 142334 6 38446 ggaren 2009-08-24 11:02:46 -0700 Comment on attachment 38446 patch > + return m_hasIntegralNamedProperty && JSObject::getOwnPropertySlot(exec, propertyName, slot); I tend to think that code like this is more readable if the m_hasIntegralNamedProperty case is an explicit early return. Up to you. r=me 142418 7 darin 2009-08-24 14:35:36 -0700 http://trac.webkit.org/changeset/47727 142419 8 darin 2009-08-24 14:38:00 -0700 http://trac.webkit.org/changeset/47730 38446 2009-08-22 23:36:59 -0700 2009-08-24 11:02:46 -0700 patch ArrayMissPatch.txt text/plain 3757 darin SW5kZXg6IEphdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBKYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDQ3Njg2KQorKysgSmF2YVNjcmlwdENvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjYgQEAKKzIwMDktMDgtMjIgIERhcmluIEFk bGVyICA8ZGFyaW5AYXBwbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgorCisgICAgICAgIEFycmF5IGluZGV4IG1pc3MgY2FzZSBjcmVhdGVzIGEgc3RyaW5nIGV2 ZXJ5IHRpbWUKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTI4NjY0CisKKyAgICAgICAgU3VuU3BpZGVyIHRlc3QgcmVzdWx0cyBJIHNhdzoKKworICAgICAg ICAgICAgMC41JSBmYXN0ZXIgb3ZlcmFsbAorICAgICAgICAgICAgMSUgZmFzdGVyIG9uIGNyeXB0 by1hZXMKKyAgICAgICAgICAgIDIwJSBmYXN0ZXIgb24gY3J5cHRvLW1kNQorICAgICAgICAgICAg MTMlIGZhc3RlciBvbiBjcnlwdG8tc2hhMQorCisgICAgICAgICogcnVudGltZS9PYmplY3RQcm90 b3R5cGUuY3BwOgorICAgICAgICAoSlNDOjpPYmplY3RQcm90b3R5cGU6Ok9iamVjdFByb3RvdHlw ZSk6IEluaXRpYWxpemUgbV9oYXNJbnRlZ3JhbE5hbWVkUHJvcGVydHkgdG8gZmFsc2UuCisgICAg ICAgIChKU0M6Ok9iamVjdFByb3RvdHlwZTo6cHV0KTogU2V0IG1faGFzSW50ZWdyYWxOYW1lZFBy b3BlcnR5IGlmIHRoZSBwcm9wZXJ0eSBoYXMgYSBuYW1lCisgICAgICAgIHRoYXQgaXMgYSAzMi1i aXQgdW5zaWduZWQgaW50ZWdlci4KKyAgICAgICAgKEpTQzo6T2JqZWN0UHJvdG90eXBlOjpnZXRP d25Qcm9wZXJ0eVNsb3QpOiBEb24ndCBjYWxsIEpTT2JqZWN0OjpnZXRPd25Qcm9wZXJ0eVNsb3QK KyAgICAgICAgdW5sZXNzIG1faGFzSW50ZWdyYWxOYW1lZFByb3BlcnR5IGlzIHRydWUgKGhpZ2hs eSB1bmxpa2VseSkuCisKKyAgICAgICAgKiBydW50aW1lL09iamVjdFByb3RvdHlwZS5oOiBBZGRl ZCBkZWNsYXJhdGlvbnMgZm9yIHRoZSBhYm92ZS4KKwogMjAwOS0wOC0yMiAgTWFjaWVqIFN0YWNo b3dpYWsgIDxtanNAYXBwbGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IE1hcmsgUm93ZS4K SW5kZXg6IEphdmFTY3JpcHRDb3JlL3J1bnRpbWUvT2JqZWN0UHJvdG90eXBlLmNwcAo9PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09Ci0tLSBKYXZhU2NyaXB0Q29yZS9ydW50aW1lL09iamVjdFByb3RvdHlwZS5jcHAJKHJldmlz aW9uIDQ3NjgzKQorKysgSmF2YVNjcmlwdENvcmUvcnVudGltZS9PYmplY3RQcm90b3R5cGUuY3Bw CSh3b3JraW5nIGNvcHkpCkBAIC00Miw2ICs0Miw3IEBAIHN0YXRpYyBKU1ZhbHVlIEpTQ19IT1NU X0NBTEwgb2JqZWN0UHJvdG8KIAogT2JqZWN0UHJvdG90eXBlOjpPYmplY3RQcm90b3R5cGUoRXhl Y1N0YXRlKiBleGVjLCBQYXNzUmVmUHRyPFN0cnVjdHVyZT4gc3R1Y3R1cmUsIFN0cnVjdHVyZSog cHJvdG90eXBlRnVuY3Rpb25TdHJ1Y3R1cmUpCiAgICAgOiBKU09iamVjdChzdHVjdHVyZSkKKyAg ICAsIG1faGFzSW50ZWdyYWxOYW1lZFByb3BlcnR5KGZhbHNlKQogewogICAgIHB1dERpcmVjdEZ1 bmN0aW9uV2l0aG91dFRyYW5zaXRpb24oZXhlYywgbmV3IChleGVjKSBOYXRpdmVGdW5jdGlvbldy YXBwZXIoZXhlYywgcHJvdG90eXBlRnVuY3Rpb25TdHJ1Y3R1cmUsIDAsIGV4ZWMtPnByb3BlcnR5 TmFtZXMoKS50b1N0cmluZywgb2JqZWN0UHJvdG9GdW5jVG9TdHJpbmcpLCBEb250RW51bSk7CiAg ICAgcHV0RGlyZWN0RnVuY3Rpb25XaXRob3V0VHJhbnNpdGlvbihleGVjLCBuZXcgKGV4ZWMpIE5h dGl2ZUZ1bmN0aW9uV3JhcHBlcihleGVjLCBwcm90b3R5cGVGdW5jdGlvblN0cnVjdHVyZSwgMCwg ZXhlYy0+cHJvcGVydHlOYW1lcygpLnRvTG9jYWxlU3RyaW5nLCBvYmplY3RQcm90b0Z1bmNUb0xv Y2FsZVN0cmluZyksIERvbnRFbnVtKTsKQEAgLTU3LDYgKzU4LDIwIEBAIE9iamVjdFByb3RvdHlw ZTo6T2JqZWN0UHJvdG90eXBlKEV4ZWNTdGEKICAgICBwdXREaXJlY3RGdW5jdGlvbldpdGhvdXRU cmFuc2l0aW9uKGV4ZWMsIG5ldyAoZXhlYykgTmF0aXZlRnVuY3Rpb25XcmFwcGVyKGV4ZWMsIHBy b3RvdHlwZUZ1bmN0aW9uU3RydWN0dXJlLCAxLCBleGVjLT5wcm9wZXJ0eU5hbWVzKCkuX19sb29r dXBTZXR0ZXJfXywgb2JqZWN0UHJvdG9GdW5jTG9va3VwU2V0dGVyKSwgRG9udEVudW0pOwogfQog Cit2b2lkIE9iamVjdFByb3RvdHlwZTo6cHV0KEV4ZWNTdGF0ZSogZXhlYywgY29uc3QgSWRlbnRp ZmllciYgcHJvcGVydHlOYW1lLCBKU1ZhbHVlIHZhbHVlLCBQdXRQcm9wZXJ0eVNsb3QmIHNsb3Qp Cit7CisgICAgSlNPYmplY3Q6OnB1dChleGVjLCBwcm9wZXJ0eU5hbWUsIHZhbHVlLCBzbG90KTsK KyAgICBib29sIGlzVUludDMyOworICAgIHByb3BlcnR5TmFtZS50b1N0cmljdFVJbnQzMigmaXNV SW50MzIpOworICAgIGlmIChpc1VJbnQzMikKKyAgICAgICAgbV9oYXNJbnRlZ3JhbE5hbWVkUHJv cGVydHkgPSB0cnVlOworfQorCitib29sIE9iamVjdFByb3RvdHlwZTo6Z2V0T3duUHJvcGVydHlT bG90KEV4ZWNTdGF0ZSogZXhlYywgdW5zaWduZWQgcHJvcGVydHlOYW1lLCBQcm9wZXJ0eVNsb3Qm IHNsb3QpCit7CisgICAgcmV0dXJuIG1faGFzSW50ZWdyYWxOYW1lZFByb3BlcnR5ICYmIEpTT2Jq ZWN0OjpnZXRPd25Qcm9wZXJ0eVNsb3QoZXhlYywgcHJvcGVydHlOYW1lLCBzbG90KTsKK30KKwog Ly8gLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tIEZ1bmN0aW9ucyAtLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLQogCiAvLyBFQ01BIDE1LjIuNC4yLCAxNS4yLjQuNCwgMTUuMi40 LjUsIDE1LjIuNC43CkluZGV4OiBKYXZhU2NyaXB0Q29yZS9ydW50aW1lL09iamVjdFByb3RvdHlw ZS5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT0KLS0tIEphdmFTY3JpcHRDb3JlL3J1bnRpbWUvT2JqZWN0UHJvdG90eXBl LmgJKHJldmlzaW9uIDQ3NjgzKQorKysgSmF2YVNjcmlwdENvcmUvcnVudGltZS9PYmplY3RQcm90 b3R5cGUuaAkod29ya2luZyBjb3B5KQpAQCAtMjgsNiArMjgsMTIgQEAgbmFtZXNwYWNlIEpTQyB7 CiAgICAgY2xhc3MgT2JqZWN0UHJvdG90eXBlIDogcHVibGljIEpTT2JqZWN0IHsKICAgICBwdWJs aWM6CiAgICAgICAgIE9iamVjdFByb3RvdHlwZShFeGVjU3RhdGUqLCBQYXNzUmVmUHRyPFN0cnVj dHVyZT4sIFN0cnVjdHVyZSogcHJvdG90eXBlRnVuY3Rpb25TdHJ1Y3R1cmUpOworCisgICAgcHJp dmF0ZToKKyAgICAgICAgdmlydHVhbCB2b2lkIHB1dChFeGVjU3RhdGUqLCBjb25zdCBJZGVudGlm aWVyJiwgSlNWYWx1ZSwgUHV0UHJvcGVydHlTbG90Jik7CisgICAgICAgIHZpcnR1YWwgYm9vbCBn ZXRPd25Qcm9wZXJ0eVNsb3QoRXhlY1N0YXRlKiwgdW5zaWduZWQgcHJvcGVydHlOYW1lLCBQcm9w ZXJ0eVNsb3QmKTsKKworICAgICAgICBib29sIG1faGFzSW50ZWdyYWxOYW1lZFByb3BlcnR5Owog ICAgIH07CiAKICAgICBKU1ZhbHVlIEpTQ19IT1NUX0NBTEwgb2JqZWN0UHJvdG9GdW5jVG9TdHJp bmcoRXhlY1N0YXRlKiwgSlNPYmplY3QqLCBKU1ZhbHVlLCBjb25zdCBBcmdMaXN0Jik7Cg==