170881 2017-04-15 21:08:47 -0700 REGRESSION(215272): microbenchmark/seal-and-do-work and microbenchmark/freeze-and-do-work are 27x slower 2017-04-18 10:54:28 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 ticaiolima msaboff fpizlo gskachkov keith_miller mark.lam msaboff saam webkit-bug-importer ysuzuki oldest_to_newest 1297725 0 ticaiolima 2017-04-15 21:08:47 -0700 Comparing r215126 with r215394 I've found the following performance regression into microbenchmarks. Need to investigate the reason. baseline changes freeze-and-do-work 17.8163+-1.1731 ! 481.5174+-10.8908 ! definitely 27.0268x slower seal-and-do-work 18.0572+-1.2018 ! 491.9421+-18.0567 ! definitely 27.2436x slower <geometric> 17.9014+-0.9676 ! 486.4490+-11.4703 ! definitely 27.1737x slower 1297775 1 msaboff 2017-04-16 10:20:22 -0700 This is like change set r215272 <https://trac.webkit.org/changeset/215272>. I'll take a look at it. 1297801 2 webkit-bug-importer 2017-04-16 13:25:28 -0700 <rdar://problem/31649374> 1298098 3 307319 msaboff 2017-04-17 16:36:07 -0700 Created attachment 307319 Patch 1298104 4 307319 saam 2017-04-17 17:09:42 -0700 Comment on attachment 307319 Patch r=me 1298108 5 307319 mark.lam 2017-04-17 17:17:38 -0700 Comment on attachment 307319 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307319&action=review > Source/JavaScriptCore/runtime/ObjectConstructor.cpp:614 > + if (isJSFinalObject(object) && !hasIndexedProperties(object->indexingType())) { > + object->seal(vm); > + return JSValue::encode(obj); > + } In r215272 <https://trac.webkit.org/changeset/215272>, you also eliminated the fast paths for objectConstructorIsSealed() and objectConstructorIsFrozen(). Should those be restored as well with the additional !hasIndexedProperties() check? 1298307 6 msaboff 2017-04-18 09:25:43 -0700 (In reply to Mark Lam from comment #5) > Comment on attachment 307319 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=307319&action=review > > > Source/JavaScriptCore/runtime/ObjectConstructor.cpp:614 > > + if (isJSFinalObject(object) && !hasIndexedProperties(object->indexingType())) { > > + object->seal(vm); > > + return JSValue::encode(obj); > > + } > > In r215272 <https://trac.webkit.org/changeset/215272>, you also eliminated > the fast paths for objectConstructorIsSealed() and > objectConstructorIsFrozen(). Should those be restored as well with the > additional !hasIndexedProperties() check? Yes. I added those checks back in. 1298352 7 msaboff 2017-04-18 10:54:28 -0700 Committed r215471: <http://trac.webkit.org/changeset/215471> 307319 2017-04-17 16:36:07 -0700 2017-04-17 17:09:42 -0700 Patch 170881.patch text/plain 1949 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjE1NDM3KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBA CisyMDE3LTA0LTE3ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIFJFR1JFU1NJT04oMjE1MjcyKTogbWljcm9iZW5jaG1hcmsvc2VhbC1hbmQtZG8td29yayBh bmQgbWljcm9iZW5jaG1hcmsvZnJlZXplLWFuZC1kby13b3JrIGFyZSAyN3ggc2xvd2VyCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNzA4ODEKKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIHJ1bnRpbWUvT2Jq ZWN0Q29uc3RydWN0b3IuY3BwOgorICAgICAgICAoSlNDOjpvYmplY3RDb25zdHJ1Y3RvclNlYWwp OgorICAgICAgICAoSlNDOjpvYmplY3RDb25zdHJ1Y3RvckZyZWV6ZSk6CisgICAgICAgIFJlc3Rv cmVkIGZhc3QgcGF0aHMgZm9yIGZpbmFsIG9iamVjdHMgdGhhdCBkb24ndCBoYXZlIGluZGV4ZWQg cHJvcGVydGllcy4KKwogMjAxNy0wNC0xNyAgWW91ZW5uIEZhYmxldCAgPHlvdWVubkBhcHBsZS5j b20+CiAKICAgICAgICAgRGlzYWJsZSBvdXRkYXRlZCBXcml0YWJsZVN0cmVhbSBBUEkKSW5kZXg6 IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL09iamVjdENvbnN0cnVjdG9yLmNwcAo9PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9PYmplY3RDb25zdHJ1Y3Rv ci5jcHAJKHJldmlzaW9uIDIxNTI3MikKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1l L09iamVjdENvbnN0cnVjdG9yLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNjA4LDYgKzYwOCwxMSBA QCBFbmNvZGVkSlNWYWx1ZSBKU0NfSE9TVF9DQUxMIG9iamVjdENvbnN0CiAgICAgICAgIHJldHVy biBKU1ZhbHVlOjplbmNvZGUob2JqKTsKICAgICBKU09iamVjdCogb2JqZWN0ID0gYXNPYmplY3Qo b2JqKTsKIAorICAgIGlmIChpc0pTRmluYWxPYmplY3Qob2JqZWN0KSAmJiAhaGFzSW5kZXhlZFBy b3BlcnRpZXMob2JqZWN0LT5pbmRleGluZ1R5cGUoKSkpIHsKKyAgICAgICAgb2JqZWN0LT5zZWFs KHZtKTsKKyAgICAgICAgcmV0dXJuIEpTVmFsdWU6OmVuY29kZShvYmopOworICAgIH0KKwogICAg IGJvb2wgc3VjY2VzcyA9IHNldEludGVncml0eUxldmVsPEludGVncml0eUxldmVsOjpTZWFsZWQ+ KGV4ZWMsIHZtLCBvYmplY3QpOwogICAgIFJFVFVSTl9JRl9FWENFUFRJT04oc2NvcGUsIGVuY29k ZWRKU1ZhbHVlKCkpOwogICAgIGlmIChVTkxJS0VMWSghc3VjY2VzcykpIHsKQEAgLTYyMyw2ICs2 MjgsMTEgQEAgSlNPYmplY3QqIG9iamVjdENvbnN0cnVjdG9yRnJlZXplKEV4ZWNTdAogICAgIFZN JiB2bSA9IGV4ZWMtPnZtKCk7CiAgICAgYXV0byBzY29wZSA9IERFQ0xBUkVfVEhST1dfU0NPUEUo dm0pOwogCisgICAgaWYgKGlzSlNGaW5hbE9iamVjdChvYmplY3QpICYmICFoYXNJbmRleGVkUHJv cGVydGllcyhvYmplY3QtPmluZGV4aW5nVHlwZSgpKSkgeworICAgICAgICBvYmplY3QtPmZyZWV6 ZSh2bSk7CisgICAgICAgIHJldHVybiBvYmplY3Q7CisgICAgfQorCiAgICAgYm9vbCBzdWNjZXNz ID0gc2V0SW50ZWdyaXR5TGV2ZWw8SW50ZWdyaXR5TGV2ZWw6OkZyb3plbj4oZXhlYywgdm0sIG9i amVjdCk7CiAgICAgUkVUVVJOX0lGX0VYQ0VQVElPTihzY29wZSwgbnVsbHB0cik7CiAgICAgaWYg KCFzdWNjZXNzKQo=