122740 2013-10-13 21:57:22 -0700 FTL should have an inefficient but correct implementation of GetById 2013-10-14 11:06:52 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 122739 1 fpizlo fpizlo atrick barraclough ggaren mark.lam mhahnenberg msaboff nrotem oliver sam oldest_to_newest 939357 0 fpizlo 2013-10-13 21:57:22 -0700 Just to expand coverage and find bugs before I implement this for real. 939360 1 214126 fpizlo 2013-10-13 22:21:38 -0700 Created attachment 214126 the patch 939367 2 nrotem 2013-10-13 23:07:45 -0700 I see two changes: 1. remove prediction 2. implement getByID Are these changes related ? What is J_JITOperation_EJI ? The line below is really long. Can you extract the identifier number into a separate variable ? 1231 setJSValue(vmCall(m_out.operation(operationGetById), m_callFrame, base, m_out.constIntPtr(m_graph.identifiers()[m_node->identifierNumber()]))); 939509 3 214126 mhahnenberg 2013-10-14 10:33:46 -0700 Comment on attachment 214126 the patch r=me 939524 4 fpizlo 2013-10-14 10:41:24 -0700 (In reply to comment #2) > I see two changes: 1. remove prediction 2. implement getByID Are these changes related ? Loosely - I implemented the FTL version of GetById by looking at what the DFG version did, but of course removing all of the inline cache stuff. Then I saw that the DFG had this assertion about predictions. I had a choice of either adding such an assertion in the FTL, or removing it. So I removed it. > > What is J_JITOperation_EJI ? It's a typedef: typedef EncodedJSValue JIT_OPERATION (*J_JITOperation_EJI)(ExecState*, EncodedJSValue, StringImpl*); See JITOperations.h. > > The line below is really long. Can you extract the identifier number into a separate variable ? > > 1231 setJSValue(vmCall(m_out.operation(operationGetById), m_callFrame, base, m_out.constIntPtr(m_graph.identifiers()[m_node->identifierNumber()]))); Ok! 939544 5 fpizlo 2013-10-14 11:06:52 -0700 Landed in http://trac.webkit.org/changeset/157409 214126 2013-10-13 22:21:38 -0700 2013-10-14 10:33:46 -0700 the patch blah.patch text/plain 6156 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTU3Mzg2KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI5IEBA CisyMDEzLTEwLTEzICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg RlRMIHNob3VsZCBoYXZlIGFuIGluZWZmaWNpZW50IGJ1dCBjb3JyZWN0IGltcGxlbWVudGF0aW9u IG9mIEdldEJ5SWQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTEyMjc0MAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorICAgICAg ICAKKyAgICAgICAgSXQgdG9vayBzb21lIGVmZm9ydCB0byByZWFsaXplIHRoYXQgdGhlIG5vZGUt PnByZWRpY3Rpb24oKSBjaGVjayBpbiB0aGUgREZHIGJhY2tlbmRzCisgICAgICAgIGFyZSBjb21w bGV0ZWx5IHVubmVjZXNzYXJ5IHNpbmNlIHRoZSBCeXRlQ29kZVBhcnNlciB3aWxsIGFsd2F5cyBp bnNlcnQgYSBGb3JjZU9TUkV4aXQKKyAgICAgICAgaWYgIXByZWRpY3Rpb24uCisgICAgICAgIAor ICAgICAgICBCdXQgb3RoZXIgdGhhbiB0aGF0IHRoaXMgd2FzIGFuIGVhc3kgcGF0Y2guCisKKyAg ICAgICAgKiBkZmcvREZHQnl0ZUNvZGVQYXJzZXIuY3BwOgorICAgICAgICAoSlNDOjpERkc6OkJ5 dGVDb2RlUGFyc2VyOjpoYW5kbGVHZXRCeUlkKToKKyAgICAgICAgKiBkZmcvREZHU3BlY3VsYXRp dmVKSVQzMl82NC5jcHA6CisgICAgICAgIChKU0M6OkRGRzo6U3BlY3VsYXRpdmVKSVQ6OmNvbXBp bGUpOgorICAgICAgICAqIGRmZy9ERkdTcGVjdWxhdGl2ZUpJVDY0LmNwcDoKKyAgICAgICAgKEpT Qzo6REZHOjpTcGVjdWxhdGl2ZUpJVDo6Y29tcGlsZSk6CisgICAgICAgICogZnRsL0ZUTENhcGFi aWxpdGllcy5jcHA6CisgICAgICAgIChKU0M6OkZUTDo6Y2FuQ29tcGlsZSk6CisgICAgICAgICog ZnRsL0ZUTEludHJpbnNpY1JlcG9zaXRvcnkuaDoKKyAgICAgICAgKiBmdGwvRlRMTG93ZXJERkdU b0xMVk0uY3BwOgorICAgICAgICAoSlNDOjpGVEw6Okxvd2VyREZHVG9MTFZNOjpjb21waWxlTm9k ZSk6CisgICAgICAgIChKU0M6OkZUTDo6TG93ZXJERkdUb0xMVk06OmNvbXBpbGVHZXRCeUlkKToK KwogMjAxMy0xMC0xMyAgRmlsaXAgUGl6bG8gIDxmcGl6bG9AYXBwbGUuY29tPgogCiAgICAgICAg IEZUTCBPU1IgZXhpdCBzaG91bGQgcGVyZm9ybSB6ZXJvIGV4dGVuc2lvbiBvbiB2YWx1ZXMgc21h bGxlciB0aGFuIDY0LWJpdApJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdCeXRl Q29kZVBhcnNlci5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9E RkdCeXRlQ29kZVBhcnNlci5jcHAJKHJldmlzaW9uIDE1NzM4NSkKKysrIFNvdXJjZS9KYXZhU2Ny aXB0Q29yZS9kZmcvREZHQnl0ZUNvZGVQYXJzZXIuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xNzY4 LDExICsxNzY4LDcgQEAgdm9pZCBCeXRlQ29kZVBhcnNlcjo6aGFuZGxlR2V0QnlJZCgKICAgICAK ICAgICBBU1NFUlQoZ2V0QnlJZFN0YXR1cy5zdHJ1Y3R1cmVTZXQoKS5zaXplKCkpOwogICAgICAg ICAgICAgICAgIAotICAgIC8vIFRoZSBpbXBsZW1lbnRhdGlvbiBvZiBHZXRCeU9mZnNldCBkb2Vz IG5vdCBrbm93IHRvIHRlcm1pbmF0ZSBzcGVjdWxhdGl2ZQotICAgIC8vIGV4ZWN1dGlvbiBpZiBp dCBkb2Vzbid0IGhhdmUgYSBwcmVkaWN0aW9uLCBzbyB3ZSBkbyBpdCBtYW51YWxseS4KLSAgICBp ZiAocHJlZGljdGlvbiA9PSBTcGVjTm9uZSkKLSAgICAgICAgYWRkVG9HcmFwaChGb3JjZU9TUkV4 aXQpOwotICAgIGVsc2UgaWYgKG1fZ3JhcGguY29tcGlsYXRpb24oKSkKKyAgICBpZiAobV9ncmFw aC5jb21waWxhdGlvbigpKQogICAgICAgICBtX2dyYXBoLmNvbXBpbGF0aW9uKCktPm5vdGljZUlu bGluZWRHZXRCeUlkKCk7CiAgICAgCiAgICAgTm9kZSogb3JpZ2luYWxCYXNlRm9yQmFzZWxpbmVK SVQgPSBiYXNlOwpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdTcGVjdWxhdGl2 ZUpJVDMyXzY0LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RG R1NwZWN1bGF0aXZlSklUMzJfNjQuY3BwCShyZXZpc2lvbiAxNTczODUpCisrKyBTb3VyY2UvSmF2 YVNjcmlwdENvcmUvZGZnL0RGR1NwZWN1bGF0aXZlSklUMzJfNjQuY3BwCSh3b3JraW5nIGNvcHkp CkBAIC0zNzc4LDEwICszNzc4LDcgQEAgdm9pZCBTcGVjdWxhdGl2ZUpJVDo6Y29tcGlsZShOb2Rl KiBub2RlKQogICAgIH0KICAgICAgICAgCiAgICAgY2FzZSBHZXRCeUlkOiB7Ci0gICAgICAgIGlm ICghbm9kZS0+cHJlZGljdGlvbigpKSB7Ci0gICAgICAgICAgICB0ZXJtaW5hdGVTcGVjdWxhdGl2 ZUV4ZWN1dGlvbihJbmFkZXF1YXRlQ292ZXJhZ2UsIEpTVmFsdWVSZWdzKCksIDApOwotICAgICAg ICAgICAgYnJlYWs7Ci0gICAgICAgIH0KKyAgICAgICAgQVNTRVJUKG5vZGUtPnByZWRpY3Rpb24o KSk7CiAgICAgICAgIAogICAgICAgICBzd2l0Y2ggKG5vZGUtPmNoaWxkMSgpLnVzZUtpbmQoKSkg ewogICAgICAgICBjYXNlIENlbGxVc2U6IHsKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9k ZmcvREZHU3BlY3VsYXRpdmVKSVQ2NC5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3Jp cHRDb3JlL2RmZy9ERkdTcGVjdWxhdGl2ZUpJVDY0LmNwcAkocmV2aXNpb24gMTU3Mzg1KQorKysg U291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdTcGVjdWxhdGl2ZUpJVDY0LmNwcAkod29ya2lu ZyBjb3B5KQpAQCAtNDA2OCwxMCArNDA2OCw3IEBAIHZvaWQgU3BlY3VsYXRpdmVKSVQ6OmNvbXBp bGUoTm9kZSogbm9kZSkKICAgICAgICAgYnJlYWs7CiAgICAgfQogICAgIGNhc2UgR2V0QnlJZDog ewotICAgICAgICBpZiAoIW5vZGUtPnByZWRpY3Rpb24oKSkgewotICAgICAgICAgICAgdGVybWlu YXRlU3BlY3VsYXRpdmVFeGVjdXRpb24oSW5hZGVxdWF0ZUNvdmVyYWdlLCBKU1ZhbHVlUmVncygp LCAwKTsKLSAgICAgICAgICAgIGJyZWFrOwotICAgICAgICB9CisgICAgICAgIEFTU0VSVChub2Rl LT5wcmVkaWN0aW9uKCkpOwogCiAgICAgICAgIHN3aXRjaCAobm9kZS0+Y2hpbGQxKCkudXNlS2lu ZCgpKSB7CiAgICAgICAgIGNhc2UgQ2VsbFVzZTogewpJbmRleDogU291cmNlL0phdmFTY3JpcHRD b3JlL2Z0bC9GVExDYXBhYmlsaXRpZXMuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2Ny aXB0Q29yZS9mdGwvRlRMQ2FwYWJpbGl0aWVzLmNwcAkocmV2aXNpb24gMTU3Mzg1KQorKysgU291 cmNlL0phdmFTY3JpcHRDb3JlL2Z0bC9GVExDYXBhYmlsaXRpZXMuY3BwCSh3b3JraW5nIGNvcHkp CkBAIC05NCw2ICs5NCw3IEBAIGlubGluZSBDYXBhYmlsaXR5TGV2ZWwgY2FuQ29tcGlsZShOb2Rl KiAKICAgICBjYXNlIEdldENsb3N1cmVSZWdpc3RlcnM6CiAgICAgY2FzZSBHZXRDbG9zdXJlVmFy OgogICAgIGNhc2UgUHV0Q2xvc3VyZVZhcjoKKyAgICBjYXNlIEdldEJ5SWQ6CiAgICAgICAgIC8v IFRoZXNlIGFyZSBPSy4KICAgICAgICAgYnJlYWs7CiAgICAgY2FzZSBHZXRJbmRleGVkUHJvcGVy dHlTdG9yYWdlOgpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2Z0bC9GVExJbnRyaW5zaWNS ZXBvc2l0b3J5LmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2Z0bC9GVExJ bnRyaW5zaWNSZXBvc2l0b3J5LmgJKHJldmlzaW9uIDE1NzM4NSkKKysrIFNvdXJjZS9KYXZhU2Ny aXB0Q29yZS9mdGwvRlRMSW50cmluc2ljUmVwb3NpdG9yeS5oCSh3b3JraW5nIGNvcHkpCkBAIC01 MSw2ICs1MSw3IEBAIG5hbWVzcGFjZSBKU0MgeyBuYW1lc3BhY2UgRlRMIHsKICNkZWZpbmUgRk9S X0VBQ0hfRlVOQ1RJT05fVFlQRShtYWNybykgXAogICAgIG1hY3JvKElfSklUT3BlcmF0aW9uX0VK c3MsIGZ1bmN0aW9uVHlwZShpbnRQdHIsIGludFB0ciwgaW50UHRyKSkgXAogICAgIG1hY3JvKEpf SklUT3BlcmF0aW9uX0UsIGZ1bmN0aW9uVHlwZShpbnQ2NCwgaW50UHRyKSkgXAorICAgIG1hY3Jv KEpfSklUT3BlcmF0aW9uX0VKSSwgZnVuY3Rpb25UeXBlKGludDY0LCBpbnRQdHIsIGludDY0LCBp bnRQdHIpKSBcCiAgICAgbWFjcm8oUF9KSVRPcGVyYXRpb25fRUMsIGZ1bmN0aW9uVHlwZShpbnRQ dHIsIGludFB0ciwgaW50UHRyKSkgXAogICAgIG1hY3JvKFZfSklUT3BlcmF0aW9uX0VPWkQsIGZ1 bmN0aW9uVHlwZSh2b2lkVHlwZSwgaW50UHRyLCBpbnRQdHIsIGludDMyLCBkb3VibGVUeXBlKSkg XAogICAgIG1hY3JvKFZfSklUT3BlcmF0aW9uX0VPWkosIGZ1bmN0aW9uVHlwZSh2b2lkVHlwZSwg aW50UHRyLCBpbnRQdHIsIGludDMyLCBpbnQ2NCkpIFwKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0 Q29yZS9mdGwvRlRMTG93ZXJERkdUb0xMVk0uY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZh U2NyaXB0Q29yZS9mdGwvRlRMTG93ZXJERkdUb0xMVk0uY3BwCShyZXZpc2lvbiAxNTczODUpCisr KyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZnRsL0ZUTExvd2VyREZHVG9MTFZNLmNwcAkod29ya2lu ZyBjb3B5KQpAQCAtMzQ4LDYgKzM0OCw5IEBAIHByaXZhdGU6CiAgICAgICAgIGNhc2UgUGhhbnRv bVB1dFN0cnVjdHVyZToKICAgICAgICAgICAgIGNvbXBpbGVQaGFudG9tUHV0U3RydWN0dXJlKCk7 CiAgICAgICAgICAgICBicmVhazsKKyAgICAgICAgY2FzZSBHZXRCeUlkOgorICAgICAgICAgICAg Y29tcGlsZUdldEJ5SWQoKTsKKyAgICAgICAgICAgIGJyZWFrOwogICAgICAgICBjYXNlIEdldEJ1 dHRlcmZseToKICAgICAgICAgICAgIGNvbXBpbGVHZXRCdXR0ZXJmbHkoKTsKICAgICAgICAgICAg IGJyZWFrOwpAQCAtMTIwOSw2ICsxMjEyLDI1IEBAIHByaXZhdGU6CiAgICAgICAgIG1fZnRsU3Rh dGUuaml0Q29kZS0+Y29tbW9uLm5vdGlmeUNvbXBpbGluZ1N0cnVjdHVyZVRyYW5zaXRpb24obV9n cmFwaC5tX3BsYW4sIGNvZGVCbG9jaygpLCBtX25vZGUpOwogICAgIH0KICAgICAKKyAgICB2b2lk IGNvbXBpbGVHZXRCeUlkKCkKKyAgICB7CisgICAgICAgIExWYWx1ZSBiYXNlID0gMDsKKworICAg ICAgICBzd2l0Y2ggKG1fbm9kZS0+Y2hpbGQxKCkudXNlS2luZCgpKSB7CisgICAgICAgIGNhc2Ug Q2VsbFVzZToKKyAgICAgICAgICAgIGJhc2UgPSBsb3dDZWxsKG1fbm9kZS0+Y2hpbGQxKCkpOwor ICAgICAgICAgICAgYnJlYWs7CisgICAgICAgIGNhc2UgVW50eXBlZFVzZToKKyAgICAgICAgICAg IGJhc2UgPSBsb3dKU1ZhbHVlKG1fbm9kZS0+Y2hpbGQxKCkpOworICAgICAgICAgICAgYnJlYWs7 CisgICAgICAgIGRlZmF1bHQ6CisgICAgICAgICAgICBSRUxFQVNFX0FTU0VSVF9OT1RfUkVBQ0hF RCgpOworICAgICAgICAgICAgYnJlYWs7CisgICAgICAgIH0KKyAgICAgICAgCisgICAgICAgIHNl dEpTVmFsdWUodm1DYWxsKG1fb3V0Lm9wZXJhdGlvbihvcGVyYXRpb25HZXRCeUlkKSwgbV9jYWxs RnJhbWUsIGJhc2UsIG1fb3V0LmNvbnN0SW50UHRyKG1fZ3JhcGguaWRlbnRpZmllcnMoKVttX25v ZGUtPmlkZW50aWZpZXJOdW1iZXIoKV0pKSk7CisgICAgfQorICAgIAogICAgIHZvaWQgY29tcGls ZUdldEJ1dHRlcmZseSgpCiAgICAgewogICAgICAgICBzZXRTdG9yYWdlKG1fb3V0LmxvYWRQdHIo bG93Q2VsbChtX25vZGUtPmNoaWxkMSgpKSwgbV9oZWFwcy5KU09iamVjdF9idXR0ZXJmbHkpKTsK