205769 2020-01-04 14:54:06 -0800 JSON.parse should lookup prototype chains during revival 2020-03-06 13:59:40 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build All All RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=208725 InRadar P2 Trivial --- 1 ashvayka ashvayka commit-queue darin ews-watchlist keith_miller mark.lam msaboff saam tzagallo webkit-bug-importer oldest_to_newest 1602787 0 ashvayka 2020-01-04 14:54:06 -0800 ECMA262: https://tc39.es/ecma262/#sec-internalizejsonproperty (step 1) Test262: https://test262.report/browse/built-ins/JSON/parse/reviver-array-get-prop-from-prototype.js https://test262.report/browse/built-ins/JSON/parse/reviver-object-get-prop-from-prototype.js Currently, JSC gets only own properties. ChakraCore fails array test, yet passes the object test. Related steps of InternalizeJSONProperty were not normatively changed since ES5. 1602790 1 386770 ashvayka 2020-01-04 15:23:04 -0800 Created attachment 386770 Patch 1603246 2 386770 darin 2020-01-06 13:28:51 -0800 Comment on attachment 386770 Patch I wanted to review this, and the change seems clean and straightforward, but I’m not enough expert on what optimization might be lost by using the get functions in the most ordinary way. My guess is that there’s no downside to this change, but I don’t feel that I am completely qualified. Someone with more recent understanding of the runtime could weigh in on that point. 1607897 3 386770 saam 2020-01-17 00:06:25 -0800 Comment on attachment 386770 Patch r=me 1607898 4 saam 2020-01-17 00:07:57 -0800 (In reply to Darin Adler from comment #2) > Comment on attachment 386770 [details] > Patch > > I wanted to review this, and the change seems clean and straightforward, but > I’m not enough expert on what optimization might be lost by using the get > functions in the most ordinary way. My guess is that there’s no downside to > this change, but I don’t feel that I am completely qualified. > > Someone with more recent understanding of the runtime could weigh in on that > point. It should be the essentially the same speed based on my reading of the code. Where it’s not is where we’re actually doing more work in order to follow the spec, when we traverse up the prototype chain searching for the property. 1608120 5 386770 ashvayka 2020-01-17 11:47:20 -0800 Comment on attachment 386770 Patch Thank you for reviews, folks. I've paused the commit queue to run some microbenchmarks: no measurable regression. Cold runs, --outer 8: TipOfTree patch json-parse-array-reviver 193.4741+-1.9814 196.1498+-1.9866 json-parse-object-reviver 218.0244+-1.2942 217.8842+-1.4113 <geometric> 205.3828+-1.6014 206.7316+-1.6745 1608132 6 386770 commit-queue 2020-01-17 12:22:03 -0800 Comment on attachment 386770 Patch Clearing flags on attachment: 386770 Committed r254757: <https://trac.webkit.org/changeset/254757> 1608133 7 commit-queue 2020-01-17 12:22:04 -0800 All reviewed patches have been landed. Closing bug. 1608134 8 webkit-bug-importer 2020-01-17 12:23:14 -0800 <rdar://problem/58691323> 386770 2020-01-04 15:23:04 -0800 2020-01-17 12:22:03 -0800 Patch bug-205769-20200105012303.patch text/plain 4873 ashvayka SW5kZXg6IEpTVGVzdHMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIEpTVGVzdHMvQ2hhbmdlTG9n CShyZXZpc2lvbiAyNTQwMzYpCisrKyBKU1Rlc3RzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpA QCAtMSwzICsxLDEyIEBACisyMDIwLTAxLTA0ICBBbGV4ZXkgU2h2YXlrYSAgPHNodmFpa2FsZXNo QGdtYWlsLmNvbT4KKworICAgICAgICBKU09OLnBhcnNlIHNob3VsZCBsb29rdXAgcHJvdG90eXBl IGNoYWlucyBkdXJpbmcgcmV2aXZhbAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9MjA1NzY5CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgKiB0ZXN0MjYyL2V4cGVjdGF0aW9ucy55YW1sOiBNYXJrIDQgdGVzdCBj YXNlcyBhcyBwYXNzaW5nLgorCiAyMDIwLTAxLTAzICBLZWl0aCBNaWxsZXIgIDxrZWl0aF9taWxs ZXJAYXBwbGUuY29tPgogCiAgICAgICAgIFVwZGF0ZSB0ZXN0MjYyIHRlc3RzIHRvIGNvbW1pdCAx NTdiMThkCkluZGV4OiBKU1Rlc3RzL3Rlc3QyNjIvZXhwZWN0YXRpb25zLnlhbWwKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gSlNUZXN0cy90ZXN0MjYyL2V4cGVjdGF0aW9ucy55YW1sCShyZXZpc2lvbiAyNTQwMzIp CisrKyBKU1Rlc3RzL3Rlc3QyNjIvZXhwZWN0YXRpb25zLnlhbWwJKHdvcmtpbmcgY29weSkKQEAg LTExMTksMTUgKzExMTksOSBAQCB0ZXN0L2J1aWx0LWlucy9GdW5jdGlvbi9wcm90b3R5cGUvdG9T dHJpCiB0ZXN0L2J1aWx0LWlucy9HZW5lcmF0b3JGdW5jdGlvbi9wcm90by1mcm9tLWN0b3ItcmVh bG0uanM6CiAgIGRlZmF1bHQ6ICdUZXN0MjYyRXJyb3I6IEV4cGVjdGVkIFNhbWVWYWx1ZSjCq1tv YmplY3QgR2VuZXJhdG9yRnVuY3Rpb25dwrssIMKrW29iamVjdCBHZW5lcmF0b3JGdW5jdGlvbl3C uykgdG8gYmUgdHJ1ZScKICAgc3RyaWN0IG1vZGU6ICdUZXN0MjYyRXJyb3I6IEV4cGVjdGVkIFNh bWVWYWx1ZSjCq1tvYmplY3QgR2VuZXJhdG9yRnVuY3Rpb25dwrssIMKrW29iamVjdCBHZW5lcmF0 b3JGdW5jdGlvbl3CuykgdG8gYmUgdHJ1ZScKLXRlc3QvYnVpbHQtaW5zL0pTT04vcGFyc2UvcmV2 aXZlci1hcnJheS1nZXQtcHJvcC1mcm9tLXByb3RvdHlwZS5qczoKLSAgZGVmYXVsdDogJ1Rlc3Qy NjJFcnJvcjogRXhwZWN0ZWQgdHJ1ZSBidXQgZ290IGZhbHNlJwotICBzdHJpY3QgbW9kZTogJ1Rl c3QyNjJFcnJvcjogRXhwZWN0ZWQgdHJ1ZSBidXQgZ290IGZhbHNlJwogdGVzdC9idWlsdC1pbnMv SlNPTi9wYXJzZS9yZXZpdmVyLWFycmF5LW5vbi1jb25maWd1cmFibGUtcHJvcC1jcmVhdGUuanM6 CiAgIGRlZmF1bHQ6ICdUZXN0MjYyRXJyb3I6IEV4cGVjdGVkIFNhbWVWYWx1ZSjCqzIywrssIMKr MsK7KSB0byBiZSB0cnVlJwogICBzdHJpY3QgbW9kZTogJ1Rlc3QyNjJFcnJvcjogRXhwZWN0ZWQg U2FtZVZhbHVlKMKrMjLCuywgwqsywrspIHRvIGJlIHRydWUnCi10ZXN0L2J1aWx0LWlucy9KU09O L3BhcnNlL3Jldml2ZXItb2JqZWN0LWdldC1wcm9wLWZyb20tcHJvdG90eXBlLmpzOgotICBkZWZh dWx0OiAnVGVzdDI2MkVycm9yOiBFeHBlY3RlZCB0cnVlIGJ1dCBnb3QgZmFsc2UnCi0gIHN0cmlj dCBtb2RlOiAnVGVzdDI2MkVycm9yOiBFeHBlY3RlZCB0cnVlIGJ1dCBnb3QgZmFsc2UnCiB0ZXN0 L2J1aWx0LWlucy9KU09OL3BhcnNlL3Jldml2ZXItb2JqZWN0LW5vbi1jb25maWd1cmFibGUtcHJv cC1jcmVhdGUuanM6CiAgIGRlZmF1bHQ6ICdUZXN0MjYyRXJyb3I6IEV4cGVjdGVkIFNhbWVWYWx1 ZSjCqzIywrssIMKrMsK7KSB0byBiZSB0cnVlJwogICBzdHJpY3QgbW9kZTogJ1Rlc3QyNjJFcnJv cjogRXhwZWN0ZWQgU2FtZVZhbHVlKMKrMjLCuywgwqsywrspIHRvIGJlIHRydWUnCkluZGV4OiBT b3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZh U2NyaXB0Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDI1NDAzMikKKysrIFNvdXJjZS9KYXZhU2Ny aXB0Q29yZS9DaGFuZ2VMb2cJKHdvcmtpbmcgY29weSkKQEAgLTEsMyArMSwyMCBAQAorMjAyMC0w MS0wNCAgQWxleGV5IFNodmF5a2EgIDxzaHZhaWthbGVzaEBnbWFpbC5jb20+CisKKyAgICAgICAg SlNPTi5wYXJzZSBzaG91bGQgbG9va3VwIHByb3RvdHlwZSBjaGFpbnMgZHVyaW5nIHJldml2YWwK KyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIwNTc2OQor CisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoaXMgcGF0 Y2ggbWFrZXMgSlNPTi5wYXJzZSB1c2UgW1tHZXRdXSBpbnN0ZWFkIG9mIFtbR2V0T3duUHJvcGVy dHldXSBkdXJpbmcgcmV2aXZhbCwKKyAgICAgICAgYWxpZ25pbmcgSlNDIHdpdGggdGhlIHNwZWMg KHN0ZXAgMSBvZiBodHRwczovL3RjMzkuZXMvZWNtYTI2Mi8jc2VjLWludGVybmFsaXplanNvbnBy b3BlcnR5KSwKKyAgICAgICAgU3BpZGVyTW9ua2V5LCBhbmQgVjguCisKKyAgICAgICAgVXNlci1w cm92aWRlZCBgcmV2aXZlcmAgY2FuIGRlbGV0ZSBwcm9wZXJ0aWVzIHRoYXQgYXJlIG5vdCB5ZXQg aW5zcGVjdGVkIGJ5IGl0c2VsZiwKKyAgICAgICAgbWFraW5nIHVzYWdlIFtbR2V0T3duUHJvcGVy dHldXSBub24tY29tcGxpYW50IHRvIHRoZSBzcGVjLgorCisgICAgICAgICogcnVudGltZS9KU09O T2JqZWN0LmNwcDoKKyAgICAgICAgKEpTQzo6V2Fsa2VyOjp3YWxrKToKKwogMjAyMC0wMS0wMiAg WXVzdWtlIFN1enVraSAgPHlzdXp1a2lAYXBwbGUuY29tPgogCiAgICAgICAgIFtKU0NdIE1hcmtl ZEJsb2NrOjpIYW5kbGUgYW5kIEJsb2NrRGlyZWN0b3J5IHNob3VsZCBiZSBzaHJ1bmsKSW5kZXg6 IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTT05PYmplY3QuY3BwCj09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0K LS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTT05PYmplY3QuY3BwCShyZXZpc2lv biAyNTQwMzIpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU09OT2JqZWN0LmNw cAkod29ya2luZyBjb3B5KQpAQCAtNjkzLDE0ICs2OTMsMTAgQEAgTkVWRVJfSU5MSU5FIEpTVmFs dWUgV2Fsa2VyOjp3YWxrKEpTVmFsdQogICAgICAgICAgICAgICAgIGlmIChpc0pTQXJyYXkoYXJy YXkpICYmIGFycmF5LT5jYW5HZXRJbmRleFF1aWNrbHkoaW5kZXgpKQogICAgICAgICAgICAgICAg ICAgICBpblZhbHVlID0gYXJyYXktPmdldEluZGV4UXVpY2tseShpbmRleCk7CiAgICAgICAgICAg ICAgICAgZWxzZSB7Ci0gICAgICAgICAgICAgICAgICAgIFByb3BlcnR5U2xvdCBzbG90KGFycmF5 LCBQcm9wZXJ0eVNsb3Q6OkludGVybmFsTWV0aG9kVHlwZTo6R2V0KTsKLSAgICAgICAgICAgICAg ICAgICAgaWYgKGFycmF5LT5tZXRob2RUYWJsZSh2bSktPmdldE93blByb3BlcnR5U2xvdEJ5SW5k ZXgoYXJyYXksIG1fZ2xvYmFsT2JqZWN0LCBpbmRleCwgc2xvdCkpCi0gICAgICAgICAgICAgICAg ICAgICAgICBpblZhbHVlID0gc2xvdC5nZXRWYWx1ZShtX2dsb2JhbE9iamVjdCwgaW5kZXgpOwot ICAgICAgICAgICAgICAgICAgICBlbHNlCi0gICAgICAgICAgICAgICAgICAgICAgICBpblZhbHVl ID0ganNVbmRlZmluZWQoKTsKKyAgICAgICAgICAgICAgICAgICAgaW5WYWx1ZSA9IGFycmF5LT5n ZXQobV9nbG9iYWxPYmplY3QsIGluZGV4KTsKICAgICAgICAgICAgICAgICAgICAgUkVUVVJOX0lG X0VYQ0VQVElPTihzY29wZSwgeyB9KTsKICAgICAgICAgICAgICAgICB9Ci0gICAgICAgICAgICAg ICAgICAgIAorCiAgICAgICAgICAgICAgICAgaWYgKGluVmFsdWUuaXNPYmplY3QoKSkgewogICAg ICAgICAgICAgICAgICAgICBzdGF0ZVN0YWNrLmFwcGVuZChBcnJheUVuZFZpc2l0TWVtYmVyKTsK ICAgICAgICAgICAgICAgICAgICAgZ290byBzdGF0ZVVua25vd247CkBAIC03NDcsMTIgKzc0Myw3 IEBAIE5FVkVSX0lOTElORSBKU1ZhbHVlIFdhbGtlcjo6d2FsayhKU1ZhbHUKICAgICAgICAgICAg ICAgICAgICAgcHJvcGVydHlTdGFjay5yZW1vdmVMYXN0KCk7CiAgICAgICAgICAgICAgICAgICAg IGJyZWFrOwogICAgICAgICAgICAgICAgIH0KLSAgICAgICAgICAgICAgICBQcm9wZXJ0eVNsb3Qg c2xvdChvYmplY3QsIFByb3BlcnR5U2xvdDo6SW50ZXJuYWxNZXRob2RUeXBlOjpHZXQpOwotICAg ICAgICAgICAgICAgIGlmIChvYmplY3QtPm1ldGhvZFRhYmxlKHZtKS0+Z2V0T3duUHJvcGVydHlT bG90KG9iamVjdCwgbV9nbG9iYWxPYmplY3QsIHByb3BlcnRpZXNbaW5kZXhdLCBzbG90KSkKLSAg ICAgICAgICAgICAgICAgICAgaW5WYWx1ZSA9IHNsb3QuZ2V0VmFsdWUobV9nbG9iYWxPYmplY3Qs IHByb3BlcnRpZXNbaW5kZXhdKTsKLSAgICAgICAgICAgICAgICBlbHNlCi0gICAgICAgICAgICAg ICAgICAgIGluVmFsdWUgPSBqc1VuZGVmaW5lZCgpOwotCisgICAgICAgICAgICAgICAgaW5WYWx1 ZSA9IG9iamVjdC0+Z2V0KG1fZ2xvYmFsT2JqZWN0LCBwcm9wZXJ0aWVzW2luZGV4XSk7CiAgICAg ICAgICAgICAgICAgLy8gVGhlIGhvbGRlciBtYXkgYmUgbW9kaWZpZWQgYnkgdGhlIHJldml2ZXIg ZnVuY3Rpb24gc28gYW55IGxvb2t1cCBtYXkgdGhyb3cKICAgICAgICAgICAgICAgICBSRVRVUk5f SUZfRVhDRVBUSU9OKHNjb3BlLCB7IH0pOwogCg==