119961 2013-08-17 14:43:45 -0700 REGRESSION(r154218): DFG::FixupPhase no longer turns GetById's child1 into CellUse 2013-08-18 21:46:07 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 119962 1 fpizlo fpizlo barraclough ggaren mark.lam mhahnenberg msaboff oliver sam oldest_to_newest 918357 0 fpizlo 2013-08-17 14:43:45 -0700 This isn't a huge deal but we should fix it. Patch forthcoming. 918358 1 209010 fpizlo 2013-08-17 14:44:36 -0700 Created attachment 209010 the patch 918359 2 209010 mhahnenberg 2013-08-17 14:45:08 -0700 Comment on attachment 209010 the patch r=me 918387 3 fpizlo 2013-08-17 22:45:34 -0700 Strangely enough, this leads to a slow-down on DSP tests. That implies that (1) those tests are heavy users of GetById and (2) they want to be able to do GetById on things that aren't cells. Weird. I'm investigating more. I don't remember seeing a progression on those tests in http://trac.webkit.org/changeset/154218; I'll test that next. 918453 4 oliver 2013-08-18 10:05:11 -0700 (In reply to comment #3) > Strangely enough, this leads to a slow-down on DSP tests. That implies that (1) those tests are heavy users of GetById and (2) they want to be able to do GetById on things that aren't cells. Weird. I'm investigating more. I don't remember seeing a progression on those tests in http://trac.webkit.org/changeset/154218; I'll test that next. Calling number prototype methods perhaps? 918454 5 fpizlo 2013-08-18 10:12:15 -0700 (In reply to comment #3) > Strangely enough, this leads to a slow-down on DSP tests. That implies that (1) those tests are heavy users of GetById and (2) they want to be able to do GetById on things that aren't cells. Weird. I'm investigating more. I don't remember seeing a progression on those tests in http://trac.webkit.org/changeset/154218; I'll test that next. Yup, that was a speed-up, and I missed it. OK, so we have some nastiness in our profiling, apparently. It shouldn't be expensive to use more speculation if we have logic to use *less* speculation instead. 918457 6 fpizlo 2013-08-18 10:14:21 -0700 (In reply to comment #4) > (In reply to comment #3) > > Strangely enough, this leads to a slow-down on DSP tests. That implies that (1) those tests are heavy users of GetById and (2) they want to be able to do GetById on things that aren't cells. Weird. I'm investigating more. I don't remember seeing a progression on those tests in http://trac.webkit.org/changeset/154218; I'll test that next. > > Calling number prototype methods perhaps? That would only explain it if we didn't also have the feature to disable cell speculation if profiling said that the input might not be a cell. 918499 7 fpizlo 2013-08-18 20:37:10 -0700 (In reply to comment #6) > (In reply to comment #4) > > (In reply to comment #3) > > > Strangely enough, this leads to a slow-down on DSP tests. That implies that (1) those tests are heavy users of GetById and (2) they want to be able to do GetById on things that aren't cells. Weird. I'm investigating more. I don't remember seeing a progression on those tests in http://trac.webkit.org/changeset/154218; I'll test that next. > > > > Calling number prototype methods perhaps? > > That would only explain it if we didn't also have the feature to disable cell speculation if profiling said that the input might not be a cell. OK, so this was the mother of all wild goose chases. It was literally the case that on the computer I was using, you got a 3% overall speed-up on DSP if you were built in a directory called "/Volumes/Data/pizlo/OpenSource". Lol. 918500 8 fpizlo 2013-08-18 20:39:10 -0700 (In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #4) > > > (In reply to comment #3) > > > > Strangely enough, this leads to a slow-down on DSP tests. That implies that (1) those tests are heavy users of GetById and (2) they want to be able to do GetById on things that aren't cells. Weird. I'm investigating more. I don't remember seeing a progression on those tests in http://trac.webkit.org/changeset/154218; I'll test that next. > > > > > > Calling number prototype methods perhaps? > > > > That would only explain it if we didn't also have the feature to disable cell speculation if profiling said that the input might not be a cell. > > OK, so this was the mother of all wild goose chases. > > It was literally the case that on the computer I was using, you got a 3% overall speed-up on DSP if you were built in a directory called "/Volumes/Data/pizlo/OpenSource". > > Lol. I'm still retesting this ... sadly it'll take a bit of time since I have to build all of WebKit to run those benchmarks. 918513 9 fpizlo 2013-08-18 21:44:57 -0700 (In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #6) > > > (In reply to comment #4) > > > > (In reply to comment #3) > > > > > Strangely enough, this leads to a slow-down on DSP tests. That implies that (1) those tests are heavy users of GetById and (2) they want to be able to do GetById on things that aren't cells. Weird. I'm investigating more. I don't remember seeing a progression on those tests in http://trac.webkit.org/changeset/154218; I'll test that next. > > > > > > > > Calling number prototype methods perhaps? > > > > > > That would only explain it if we didn't also have the feature to disable cell speculation if profiling said that the input might not be a cell. > > > > OK, so this was the mother of all wild goose chases. > > > > It was literally the case that on the computer I was using, you got a 3% overall speed-up on DSP if you were built in a directory called "/Volumes/Data/pizlo/OpenSource". > > > > Lol. > > I'm still retesting this ... sadly it'll take a bit of time since I have to build all of WebKit to run those benchmarks. Yeah. Total fluke. 918514 10 fpizlo 2013-08-18 21:46:07 -0700 Landed in http://trac.webkit.org/changeset/154261 209010 2013-08-17 14:44:36 -0700 2013-08-17 14:45:07 -0700 the patch blah.patch text/plain 1125 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTU0MjQwKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBA CisyMDEzLTA4LTE3ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg UkVHUkVTU0lPTihyMTU0MjE4KTogREZHOjpGaXh1cFBoYXNlIG5vIGxvbmdlciB0dXJucyBHZXRC eUlkJ3MgY2hpbGQxIGludG8gQ2VsbFVzZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9y Zy9zaG93X2J1Zy5jZ2k/aWQ9MTE5OTYxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChP T1BTISkuCisKKyAgICAgICAgKiBkZmcvREZHRml4dXBQaGFzZS5jcHA6CisgICAgICAgIChKU0M6 OkRGRzo6Rml4dXBQaGFzZTo6Zml4dXBOb2RlKToKKwogMjAxMy0wOC0xNiAgRmlsaXAgUGl6bG8g IDxmcGl6bG9AYXBwbGUuY29tPgogCiAgICAgICAgIERGRyBzaG91bGQgb3B0aW1pemUgdHlwZWRB cnJheS5ieXRlTGVuZ3RoCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR0ZpeHVw UGhhc2UuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcvREZHRml4 dXBQaGFzZS5jcHAJKHJldmlzaW9uIDE1NDIzOSkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9k ZmcvREZHRml4dXBQaGFzZS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTczNyw2ICs3MzcsNyBAQCBw cml2YXRlOgogICAgICAgICAgICAgICAgIGF0dGVtcHRUb01ha2VHZXRCeXRlTGVuZ3RoKG5vZGUp OwogICAgICAgICAgICAgICAgIGJyZWFrOwogICAgICAgICAgICAgfQorICAgICAgICAgICAgc2V0 VXNlS2luZEFuZFVuYm94SWZQcm9maXRhYmxlPENlbGxVc2U+KG5vZGUtPmNoaWxkMSgpKTsKICAg ICAgICAgICAgIGJyZWFrOwogICAgICAgICB9CiAgICAgICAgICAgICAK