38988 2010-05-12 06:22:05 -0700 Avoid to store the beginning of the match on the stack in Yarr JIT 2010-06-16 01:54:49 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 0 pvarga webkit-unassigned abecsi barraclough eric ggaren zherczeg oldest_to_newest 224565 0 pvarga 2010-05-12 06:22:05 -0700 The current JIT solution stores and changes the start character position of the whole pattern's match on the stack. To store the beginning of the match directly in the output array can be a better solution because this value doesn't need to be available by the use of the stack pointer. 224568 1 55838 pvarga 2010-05-12 06:25:47 -0700 Created attachment 55838 proposed patch 237463 2 eric 2010-06-12 21:01:54 -0700 Ping? Any YARR JIT reviewers care able to review this simple patch? 237957 3 ggaren 2010-06-14 11:50:15 -0700 Peter, can you post performance test results? For this patch, I would test SunSpider and v8-regexp. 237958 4 55838 ggaren 2010-06-14 11:50:46 -0700 Comment on attachment 55838 proposed patch Code looks good, but I'm going to say r- for now, waiting on performance numbers. 238418 5 55838 pvarga 2010-06-15 09:31:15 -0700 Comment on attachment 55838 proposed patch We don't need to do this allocation on stack, so this patch solves the removed TODO. It saves sizeof(int) bytes from stack per matching. In fact, this patch doesn't even have effect on performance, it improves the memory usage. Btw, the performance results are: ref mod regexp-dna: 22.4ms +/- 1.6% 22.4ms +/- 1.6% v8-regexp: 445.9ms +/- 2.6% 444.5ms +/- 2.6% I set to r? again. 238422 6 55838 ggaren 2010-06-15 09:47:32 -0700 Comment on attachment 55838 proposed patch r=me 238732 7 55838 pvarga 2010-06-16 01:42:53 -0700 Comment on attachment 55838 proposed patch Thanks Geoffrey. cq removed to land it manually. 238735 8 55838 abecsi 2010-06-16 01:53:17 -0700 Comment on attachment 55838 proposed patch Sending JavaScriptCore/ChangeLog Sending JavaScriptCore/yarr/RegexJIT.cpp Transmitting file data .. Committed revision 61244. Clearing flags. (In reply to comment #7) > (From update of attachment 55838 [details]) > Thanks Geoffrey. > cq removed to land it manually. 238737 9 abecsi 2010-06-16 01:54:49 -0700 All reviewed patches landed. Closing bug. 55838 2010-05-12 06:25:47 -0700 2010-06-16 01:53:17 -0700 proposed patch 0001-avoid-to-store-matchBegin-on-the-stack.patch text/plain 4370 pvarga RnJvbSAyMmYxMzQxYmQwMTkzOWIxYjBlMWZhOGU0YmM3YzQ3NTkzM2YwMTEwIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBQZXRlciBWYXJnYSA8cHZhcmdhQGluZi51LXN6ZWdlZC5odT4K RGF0ZTogV2VkLCAxMiBNYXkgMjAxMCAxNToyNDozMyArMDIwMApTdWJqZWN0OiBbUEFUQ0hdIGF2 b2lkIHRvIHN0b3JlIG1hdGNoQmVnaW4gb24gdGhlIHN0YWNrCgotLS0KIEphdmFTY3JpcHRDb3Jl L0NoYW5nZUxvZyAgICAgICAgIHwgICAxMSArKysrKysrKysrKwogSmF2YVNjcmlwdENvcmUveWFy ci9SZWdleEpJVC5jcHAgfCAgIDI0ICsrKysrKysrKysrLS0tLS0tLS0tLS0tLQogMiBmaWxlcyBj aGFuZ2VkLCAyMiBpbnNlcnRpb25zKCspLCAxMyBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9K YXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cgYi9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKaW5kZXgg Y2I5OTMyZS4uODU0NWM1OSAxMDA2NDQKLS0tIGEvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCisr KyBiL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE2IEBACiAyMDEwLTA1LTEy ICBQZXRlciBWYXJnYSAgPHB2YXJnYUBpbmYudS1zemVnZWQuaHU+CiAKKyAgICAgICAgUmV2aWV3 ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgU3RvcmUgbWF0Y2hCZWdpbiBkaXJlY3Rs eSBpbiB0aGUgYXJyYXkgb2Ygb3V0cHV0IGluc3RlYWQgb2YgdGhlIHN0YWNrLgorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Mzg5ODgKKworICAgICAgICAq IHlhcnIvUmVnZXhKSVQuY3BwOgorICAgICAgICAoSlNDOjpZYXJyOjpSZWdleEdlbmVyYXRvcjo6 Z2VuZXJhdGVEaXNqdW5jdGlvbik6CisgICAgICAgIChKU0M6OllhcnI6OlJlZ2V4R2VuZXJhdG9y OjpnZW5lcmF0ZSk6CisKKzIwMTAtMDUtMTIgIFBldGVyIFZhcmdhICA8cHZhcmdhQGluZi51LXN6 ZWdlZC5odT4KKwogICAgICAgICBSZXZpZXdlZCBieSBHYXZpbiBCYXJyYWNsb3VnaC4KIAogICAg ICAgICBSZW1vdmUgYSBkaXJlY3QganVtcCBpbiBnZW5lcmF0ZVBhdHRlcm5DaGFyYWN0ZXJHcmVl ZHkoKSBhbmQKZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL3lhcnIvUmVnZXhKSVQuY3BwIGIv SmF2YVNjcmlwdENvcmUveWFyci9SZWdleEpJVC5jcHAKaW5kZXggZTMzZGJhMC4uYjUxYTFhMyAx MDA2NDQKLS0tIGEvSmF2YVNjcmlwdENvcmUveWFyci9SZWdleEpJVC5jcHAKKysrIGIvSmF2YVNj cmlwdENvcmUveWFyci9SZWdleEpJVC5jcHAKQEAgLTExNTksMTYgKzExNTksMTggQEAgY2xhc3Mg UmVnZXhHZW5lcmF0b3IgOiBwcml2YXRlIE1hY3JvQXNzZW1ibGVyIHsKICAgICAgICAgICAgIC8v IElmIHdlIGdldCBoZXJlLCB0aGUgYWx0ZXJuYXRpdmUgbWF0Y2hlZC4KICAgICAgICAgICAgIGlm IChtX3BhdHRlcm4ubV9ib2R5LT5tX2NhbGxGcmFtZVNpemUpCiAgICAgICAgICAgICAgICAgYWRk UHRyKEltbTMyKG1fcGF0dGVybi5tX2JvZHktPm1fY2FsbEZyYW1lU2l6ZSAqIHNpemVvZih2b2lk KikpLCBzdGFja1BvaW50ZXJSZWdpc3Rlcik7Ci0gICAgICAgICAgICAKKwogICAgICAgICAgICAg QVNTRVJUKGluZGV4ICE9IHJldHVyblJlZ2lzdGVyKTsKICAgICAgICAgICAgIGlmIChtX3BhdHRl cm4ubV9ib2R5LT5tX2hhc0ZpeGVkU2l6ZSkgewogICAgICAgICAgICAgICAgIG1vdmUoaW5kZXgs IHJldHVyblJlZ2lzdGVyKTsKICAgICAgICAgICAgICAgICBpZiAoYWx0ZXJuYXRpdmUtPm1fbWlu aW11bVNpemUpCiAgICAgICAgICAgICAgICAgICAgIHN1YjMyKEltbTMyKGFsdGVybmF0aXZlLT5t X21pbmltdW1TaXplKSwgcmV0dXJuUmVnaXN0ZXIpOworCisgICAgICAgICAgICAgICAgc3RvcmUz MihyZXR1cm5SZWdpc3Rlciwgb3V0cHV0KTsKICAgICAgICAgICAgIH0gZWxzZQotICAgICAgICAg ICAgICAgIHBvcChyZXR1cm5SZWdpc3Rlcik7CisgICAgICAgICAgICAgICAgbG9hZDMyKEFkZHJl c3Mob3V0cHV0KSwgcmV0dXJuUmVnaXN0ZXIpOworCiAgICAgICAgICAgICBzdG9yZTMyKGluZGV4 LCBBZGRyZXNzKG91dHB1dCwgNCkpOwotICAgICAgICAgICAgc3RvcmUzMihyZXR1cm5SZWdpc3Rl ciwgb3V0cHV0KTsKIAogICAgICAgICAgICAgZ2VuZXJhdGVSZXR1cm4oKTsKIApAQCAtMTI1Miw3 ICsxMjU0LDcgQEAgY2xhc3MgUmVnZXhHZW5lcmF0b3IgOiBwcml2YXRlIE1hY3JvQXNzZW1ibGVy IHsKICAgICAgICAgICAgIGlmICghbV9wYXR0ZXJuLm1fYm9keS0+bV9oYXNGaXhlZFNpemUpIHsK ICAgICAgICAgICAgICAgICBtb3ZlKGluZGV4LCByZWdUMCk7CiAgICAgICAgICAgICAgICAgc3Vi MzIoSW1tMzIoY291bnRDaGVja2VkRm9yQ3VycmVudEFsdGVybmF0aXZlIC0gMSksIHJlZ1QwKTsK LSAgICAgICAgICAgICAgICBwb2tlKHJlZ1QwLCBtX3BhdHRlcm4ubV9ib2R5LT5tX2NhbGxGcmFt ZVNpemUpOworICAgICAgICAgICAgICAgIHN0b3JlMzIocmVnVDAsIEFkZHJlc3Mob3V0cHV0KSk7 CiAgICAgICAgICAgICB9CiAKICAgICAgICAgICAgIC8vIFVwZGF0ZSBpbmRleCBpZiBuZWNlc3Nh cnksIGFuZCBsb29wICh3aXRob3V0IGNoZWNraW5nKS4KQEAgLTEyNjcsOSArMTI2OSw5IEBAIGNs YXNzIFJlZ2V4R2VuZXJhdG9yIDogcHJpdmF0ZSBNYWNyb0Fzc2VtYmxlciB7CiAgICAgICAgICAg ICBpZiAoY291bnRDaGVja2VkRm9yQ3VycmVudEFsdGVybmF0aXZlIC0gMSkgewogICAgICAgICAg ICAgICAgIG1vdmUoaW5kZXgsIHJlZ1QwKTsKICAgICAgICAgICAgICAgICBzdWIzMihJbW0zMihj b3VudENoZWNrZWRGb3JDdXJyZW50QWx0ZXJuYXRpdmUgLSAxKSwgcmVnVDApOwotICAgICAgICAg ICAgICAgIHBva2UocmVnVDAsIG1fcGF0dGVybi5tX2JvZHktPm1fY2FsbEZyYW1lU2l6ZSk7Cisg ICAgICAgICAgICAgICAgc3RvcmUzMihyZWdUMCwgQWRkcmVzcyhvdXRwdXQpKTsKICAgICAgICAg ICAgIH0gZWxzZQotICAgICAgICAgICAgICAgIHBva2UoaW5kZXgsIG1fcGF0dGVybi5tX2JvZHkt Pm1fY2FsbEZyYW1lU2l6ZSk7CisgICAgICAgICAgICAgICAgc3RvcmUzMihpbmRleCwgQWRkcmVz cyhvdXRwdXQpKTsKICAgICAgICAgfQogICAgICAgICAvLyBDaGVjayBpZiB0aGVyZSBpcyBzdWZm aWNlbnQgaW5wdXQgdG8gcnVuIHRoZSBmaXJzdCBhbHRlcm5hdGl2ZSBhZ2Fpbi4KICAgICAgICAg anVtcElmQXZhaWxhYmxlSW5wdXQoaW5jcmVtZW50Rm9yTmV4dEl0ZXIpLmxpbmtUbyhmaXJzdEFs dGVybmF0aXZlSW5wdXRDaGVja2VkLCB0aGlzKTsKQEAgLTEyOTUsMTEgKzEyOTcsOCBAQCBjbGFz cyBSZWdleEdlbmVyYXRvciA6IHByaXZhdGUgTWFjcm9Bc3NlbWJsZXIgewogICAgICAgICAvLyBp dCBoYXMgZWl0aGVyIGJlZW4gaW5jcmVtZW50ZWQgYnkgMSBvciBieSAoY291bnRUb0NoZWNrRm9y Rmlyc3RBbHRlcm5hdGl2ZSArIDEpIC4uLiAKICAgICAgICAgLy8gYnV0IHNpbmNlIHdlJ3JlIGFi b3V0IHRvIHJldHVybiBhIGZhaWx1cmUgdGhpcyBkb2Vzbid0IHJlYWxseSBtYXR0ZXIhKQogCi0g ICAgICAgIHVuc2lnbmVkIGZyYW1lU2l6ZSA9IG1fcGF0dGVybi5tX2JvZHktPm1fY2FsbEZyYW1l U2l6ZTsKLSAgICAgICAgaWYgKCFtX3BhdHRlcm4ubV9ib2R5LT5tX2hhc0ZpeGVkU2l6ZSkKLSAg ICAgICAgICAgICsrZnJhbWVTaXplOwotICAgICAgICBpZiAoZnJhbWVTaXplKQotICAgICAgICAg ICAgYWRkUHRyKEltbTMyKGZyYW1lU2l6ZSAqIHNpemVvZih2b2lkKikpLCBzdGFja1BvaW50ZXJS ZWdpc3Rlcik7CisgICAgICAgIGlmIChtX3BhdHRlcm4ubV9ib2R5LT5tX2NhbGxGcmFtZVNpemUp CisgICAgICAgICAgICBhZGRQdHIoSW1tMzIobV9wYXR0ZXJuLm1fYm9keS0+bV9jYWxsRnJhbWVT aXplICogc2l6ZW9mKHZvaWQqKSksIHN0YWNrUG9pbnRlclJlZ2lzdGVyKTsKIAogICAgICAgICBt b3ZlKEltbTMyKC0xKSwgcmV0dXJuUmVnaXN0ZXIpOwogCkBAIC0xMzY4LDkgKzEzNjcsOCBAQCBw dWJsaWM6CiAgICAgewogICAgICAgICBnZW5lcmF0ZUVudGVyKCk7CiAKLSAgICAgICAgLy8gVE9E TzogZG8gSSByZWFsbHkgd2FudCB0aGlzIG9uIHRoZSBzdGFjaz8KICAgICAgICAgaWYgKCFtX3Bh dHRlcm4ubV9ib2R5LT5tX2hhc0ZpeGVkU2l6ZSkKLSAgICAgICAgICAgIHB1c2goaW5kZXgpOwor ICAgICAgICAgICAgc3RvcmUzMihpbmRleCwgQWRkcmVzcyhvdXRwdXQpKTsKIAogICAgICAgICBp ZiAobV9wYXR0ZXJuLm1fYm9keS0+bV9jYWxsRnJhbWVTaXplKQogICAgICAgICAgICAgc3ViUHRy KEltbTMyKG1fcGF0dGVybi5tX2JvZHktPm1fY2FsbEZyYW1lU2l6ZSAqIHNpemVvZih2b2lkKikp LCBzdGFja1BvaW50ZXJSZWdpc3Rlcik7Ci0tIAoxLjYuNC40Cgo=