101299 2012-11-05 21:10:18 -0800 DFG should not fall down to patchable GetById just because a prototype had things added to it 2012-11-05 23:51:06 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 fpizlo fpizlo barraclough ggaren mark.lam mhahnenberg msaboff oliver sam oldest_to_newest 759217 0 fpizlo 2012-11-05 21:10:18 -0800 Currently StructureTransitionWatchpoints have a BadCache exit profile. This is often a good thing - for example if we turned the CheckStructure of the base of an access into a StructureTransitionWatchpoint, and then the watchpoint was fired because there was an unexpected side-effect. It's fair to assume that side effect might happen again, and so we ought to be conservative. But if we set a watchpoint on the structure of the prototype, and that watchpoint fired, then we shouldn't necessarily fall off into using a patchable GetById. Most likely, the reason why the watchpoint fired was just a modification to the prototype. If such modifications happen frequently then the baseline JIT's get_by_id will go polymorphic and/or have a high slow path count - so the fact that the watchpoint fired doesn't mean we can't set the watchpoint again on the next recompile, if everything else tells us it is safe to do so. 759226 1 172483 fpizlo 2012-11-05 21:18:25 -0800 Created attachment 172483 the patch 759231 2 172483 ggaren 2012-11-05 21:32:05 -0800 Comment on attachment 172483 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=172483&action=review r=me > Source/JavaScriptCore/ChangeLog:8 > + Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!). Boo. 759234 3 fpizlo 2012-11-05 21:42:31 -0800 (In reply to comment #2) > (From update of attachment 172483 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=172483&action=review > > r=me > > > Source/JavaScriptCore/ChangeLog:8 > > + Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!). > > Boo. Oops. Forgot to save my ChangeLog. Text is: DFG should not fall down to patchable GetById just because a prototype had things added to it https://bugs.webkit.org/show_bug.cgi?id=101299 Reviewed by NOBODY (OOPS!). This looks like a slight win on V8v7 and SunSpider. * bytecode/DFGExitProfile.h: (JSC::DFG::exitKindToString): * dfg/DFGSpeculativeJIT64.cpp: (JSC::DFG::SpeculativeJIT::compile): 759315 4 fpizlo 2012-11-05 23:51:06 -0800 Landed in http://trac.webkit.org/changeset/133567 172483 2012-11-05 21:18:25 -0800 2012-11-05 21:32:05 -0800 the patch betterwatch.patch text/plain 4026 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTMzNTU3KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBA CisyMDEyLTExLTA1ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg REZHIHNob3VsZCBub3QgZmFsbCBkb3duIHRvIHBhdGNoYWJsZSBHZXRCeUlkIGp1c3QgYmVjYXVz ZSBhIHByb3RvdHlwZSBoYWQgdGhpbmdzIGFkZGVkIHRvIGl0CisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMDEyOTkKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBBZGRpdGlvbmFsIGluZm9ybWF0aW9uIG9mIHRo ZSBjaGFuZ2Ugc3VjaCBhcyBhcHByb2FjaCwgcmF0aW9uYWxlLiBQbGVhc2UgYWRkIHBlci1mdW5j dGlvbiBkZXNjcmlwdGlvbnMgYmVsb3cgKE9PUFMhKS4KKworICAgICAgICAqIGJ5dGVjb2RlL0RG R0V4aXRQcm9maWxlLmg6CisgICAgICAgIChKU0M6OkRGRzo6ZXhpdEtpbmRUb1N0cmluZyk6Cisg ICAgICAgICogZGZnL0RGR1NwZWN1bGF0aXZlSklUNjQuY3BwOgorICAgICAgICAoSlNDOjpERkc6 OlNwZWN1bGF0aXZlSklUOjpjb21waWxlKToKKwogMjAxMi0xMS0wNSAgWXVxaWFuZyBYaWFuICA8 eXVxaWFuZy54aWFuQGludGVsLmNvbT4KIAogICAgICAgICBSZWZhY3RvciBMTEludDY0IHRvIGRp c3Rpbmd1aXNoIHRoZSBwb2ludGVyIG9wZXJhdGlvbnMgZnJvbSB0aGUgNjQtYml0IGludGVnZXIg b3BlcmF0aW9ucwpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2J5dGVjb2RlL0RGR0V4aXRQ cm9maWxlLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2J5dGVjb2RlL0RG R0V4aXRQcm9maWxlLmgJKHJldmlzaW9uIDEzMzU1MCkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29y ZS9ieXRlY29kZS9ERkdFeGl0UHJvZmlsZS5oCSh3b3JraW5nIGNvcHkpCkBAIC0zNiw2ICszNiw3 IEBAIGVudW0gRXhpdEtpbmQgewogICAgIEV4aXRLaW5kVW5zZXQsCiAgICAgQmFkVHlwZSwgLy8g V2UgZXhpdGVkIGJlY2F1c2UgYSB0eXBlIHByZWRpY3Rpb24gd2FzIHdyb25nLgogICAgIEJhZENh Y2hlLCAvLyBXZSBleGl0ZWQgYmVjYXVzZSBhbiBpbmxpbmUgY2FjaGUgd2FzIHdyb25nLgorICAg IEJhZFdlYWtDb25zdGFudENhY2hlLCAvLyBXZSBleGl0ZWQgYmVjYXVzZSBhIGNhY2hlIG9uIGEg d2VhayBjb25zdGFudCAodXN1YWxseSBhIHByb3RvdHlwZSkgd2FzIHdyb25nLgogICAgIEJhZElu ZGV4aW5nVHlwZSwgLy8gV2UgZXhpdGVkIGJlY2F1c2UgYW4gaW5kZXhpbmcgdHlwZSB3YXMgd3Jv bmcuCiAgICAgT3ZlcmZsb3csIC8vIFdlIGV4aXRlZCBiZWNhdXNlIG9mIG92ZXJmbG93LgogICAg IE5lZ2F0aXZlWmVybywgLy8gV2UgZXhpdGVkIGJlY2F1c2Ugd2UgZW5jb3VudGVyZWQgbmVnYXRp dmUgemVyby4KQEAgLTU1LDYgKzU2LDggQEAgaW5saW5lIGNvbnN0IGNoYXIqIGV4aXRLaW5kVG9T dHJpbmcoRXhpdAogICAgICAgICByZXR1cm4gIkJhZFR5cGUiOwogICAgIGNhc2UgQmFkQ2FjaGU6 CiAgICAgICAgIHJldHVybiAiQmFkQ2FjaGUiOworICAgIGNhc2UgQmFkV2Vha0NvbnN0YW50Q2Fj aGU6CisgICAgICAgIHJldHVybiAiQmFkV2Vha0NvbnN0YW50Q2FjaGUiOwogICAgIGNhc2UgT3Zl cmZsb3c6CiAgICAgICAgIHJldHVybiAiT3ZlcmZsb3ciOwogICAgIGNhc2UgTmVnYXRpdmVaZXJv OgpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdTcGVjdWxhdGl2ZUpJVDY0LmNw cAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR1NwZWN1bGF0aXZl SklUNjQuY3BwCShyZXZpc2lvbiAxMzM1NTApCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZn L0RGR1NwZWN1bGF0aXZlSklUNjQuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0zODMwLDkgKzM4MzAs MTUgQEAgdm9pZCBTcGVjdWxhdGl2ZUpJVDo6Y29tcGlsZShOb2RlJiBub2RlKQogICAgICAgICAK ICAgICAgICAgQVNTRVJUKG5vZGUuc3RydWN0dXJlU2V0KCkuc2l6ZSgpKTsKICAgICAgICAgCisg ICAgICAgIEV4aXRLaW5kIGV4aXRLaW5kOworICAgICAgICBpZiAobV9qaXQuZ3JhcGgoKVtub2Rl LmNoaWxkMSgpXS5vcCgpID09IFdlYWtKU0NvbnN0YW50KQorICAgICAgICAgICAgZXhpdEtpbmQg PSBCYWRXZWFrQ29uc3RhbnRDYWNoZTsKKyAgICAgICAgZWxzZQorICAgICAgICAgICAgZXhpdEtp bmQgPSBCYWRDYWNoZTsKKyAgICAgICAgCiAgICAgICAgIGlmIChub2RlLnN0cnVjdHVyZVNldCgp LnNpemUoKSA9PSAxKSB7CiAgICAgICAgICAgICBzcGVjdWxhdGlvbkNoZWNrV2l0aENvbmRpdGlv bmFsRGlyZWN0aW9uKAotICAgICAgICAgICAgICAgIEJhZENhY2hlLCBKU1ZhbHVlUmVncyhiYXNl LmdwcigpKSwgTm9Ob2RlLAorICAgICAgICAgICAgICAgIGV4aXRLaW5kLCBKU1ZhbHVlUmVncyhi YXNlLmdwcigpKSwgTm9Ob2RlLAogICAgICAgICAgICAgICAgIG1faml0LmJyYW5jaFdlYWtQdHIo CiAgICAgICAgICAgICAgICAgICAgIEpJVENvbXBpbGVyOjpOb3RFcXVhbCwKICAgICAgICAgICAg ICAgICAgICAgSklUQ29tcGlsZXI6OkFkZHJlc3MoYmFzZS5ncHIoKSwgSlNDZWxsOjpzdHJ1Y3R1 cmVPZmZzZXQoKSksCkBAIC0zODQ5LDcgKzM4NTUsNyBAQCB2b2lkIFNwZWN1bGF0aXZlSklUOjpj b21waWxlKE5vZGUmIG5vZGUpCiAgICAgICAgICAgICAgICAgZG9uZS5hcHBlbmQobV9qaXQuYnJh bmNoV2Vha1B0cihKSVRDb21waWxlcjo6RXF1YWwsIHN0cnVjdHVyZS5ncHIoKSwgbm9kZS5zdHJ1 Y3R1cmVTZXQoKVtpXSkpOwogICAgICAgICAgICAgCiAgICAgICAgICAgICBzcGVjdWxhdGlvbkNo ZWNrV2l0aENvbmRpdGlvbmFsRGlyZWN0aW9uKAotICAgICAgICAgICAgICAgIEJhZENhY2hlLCBK U1ZhbHVlUmVncyhiYXNlLmdwcigpKSwgTm9Ob2RlLAorICAgICAgICAgICAgICAgIGV4aXRLaW5k LCBKU1ZhbHVlUmVncyhiYXNlLmdwcigpKSwgTm9Ob2RlLAogICAgICAgICAgICAgICAgIG1faml0 LmJyYW5jaFdlYWtQdHIoCiAgICAgICAgICAgICAgICAgICAgIEpJVENvbXBpbGVyOjpOb3RFcXVh bCwgc3RydWN0dXJlLmdwcigpLCBub2RlLnN0cnVjdHVyZVNldCgpLmxhc3QoKSksCiAgICAgICAg ICAgICAgICAgbm9kZS5vcCgpID09IEZvcndhcmRDaGVja1N0cnVjdHVyZSk7CkBAIC0zODczLDcg KzM4NzksOCBAQCB2b2lkIFNwZWN1bGF0aXZlSklUOjpjb21waWxlKE5vZGUmIG5vZGUpCiAgICAg ICAgIG1faml0LmFkZFdlYWtSZWZlcmVuY2Uobm9kZS5zdHJ1Y3R1cmUoKSk7CiAgICAgICAgIG5v ZGUuc3RydWN0dXJlKCktPmFkZFRyYW5zaXRpb25XYXRjaHBvaW50KAogICAgICAgICAgICAgc3Bl Y3VsYXRpb25XYXRjaHBvaW50V2l0aENvbmRpdGlvbmFsRGlyZWN0aW9uKAotICAgICAgICAgICAg ICAgIEJhZENhY2hlLCBub2RlLm9wKCkgPT0gRm9yd2FyZFN0cnVjdHVyZVRyYW5zaXRpb25XYXRj aHBvaW50KSk7CisgICAgICAgICAgICAgICAgbV9qaXQuZ3JhcGgoKVtub2RlLmNoaWxkMSgpXS5v cCgpID09IFdlYWtKU0NvbnN0YW50ID8gQmFkV2Vha0NvbnN0YW50Q2FjaGUgOiBCYWRDYWNoZSwK KyAgICAgICAgICAgICAgICBub2RlLm9wKCkgPT0gRm9yd2FyZFN0cnVjdHVyZVRyYW5zaXRpb25X YXRjaHBvaW50KSk7CiAKICNpZiAhQVNTRVJUX0RJU0FCTEVECiAgICAgICAgIFNwZWN1bGF0ZUNl bGxPcGVyYW5kIG9wMSh0aGlzLCBub2RlLmNoaWxkMSgpKTsK