162001 2016-09-14 17:11:58 -0700 TextBreakIterator: unconvolute character break cache 2016-09-15 13:50:24 -0700 1 1 1 Unclassified WebKit Web Template Framework WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jfbastien jfbastien benjamin cdumez cmarcelo commit-queue dbates mmaxfield simon.fraser oldest_to_newest 1229837 0 jfbastien 2016-09-14 17:11:58 -0700 TextBreakIterator's character break cache is weird. Added here (fixed a bit after): https://bugs.webkit.org/attachment.cgi?id=144118&action=prettypatch Updated to never use a lock, and always use weak cmpxchg here: https://bugs.webkit.org/attachment.cgi?id=261719&action=prettypatch It's just trying to reduce the number of times it calls into ICU to initialize a UBRK_CHARACTER. The implementation keeps a one-element cache of the latest used one, which threads can optimistically grab. Failure is fine (just create a new one), same for failure to cache (just destroy it), but the logic is odd and you technically need release / acquire semantics because the UBRK_CHARACTER creation's store need to be visible on acquisition (but realistically it was created and then used and *then* cached, so it's probably been long ago enough that read reorders never manifested). 1229841 1 288901 jfbastien 2016-09-14 17:16:45 -0700 Created attachment 288901 patch Note that this still leaks any cached instance on shutdown, but it didn't matter before so likely doesn't now. It's also not a good fit for std::once_flag + std::call_once because IIUC multiple threads can't use the instance concurrently (the cache just passes it around, loaning it to *one* thread at a time). 1230153 2 288901 msaboff 2016-09-15 13:23:01 -0700 Comment on attachment 288901 patch r=me 1230164 3 288901 commit-queue 2016-09-15 13:46:07 -0700 Comment on attachment 288901 patch Clearing flags on attachment: 288901 Committed r205995: <http://trac.webkit.org/changeset/205995> 1230165 4 commit-queue 2016-09-15 13:46:12 -0700 All reviewed patches have been landed. Closing bug. 288901 2016-09-14 17:16:45 -0700 2016-09-15 13:46:07 -0700 patch 0001-exchange.patch text/plain 4908 jfbastien RnJvbSA2ODU1ZDViNzZmM2NiY2Q2MzlhZThjN2RjMjI1Y2IwMDM0MjFlMGU0IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBKRiBCYXN0aWVuIDxqZmJhc3RpZW5AYXBwbGUuY29tPgpEYXRl OiBXZWQsIDE0IFNlcCAyMDE2IDE3OjEyOjExIC0wNzAwClN1YmplY3Q6IFtQQVRDSF0gZXhjaGFu Z2UKCi0tLQogU291cmNlL1dURi9DaGFuZ2VMb2cgICAgICAgICAgICAgICAgICAgICAgfCAzMiAr KysrKysrKysrKysrKysrKysrKysrKysrKysrKysrCiBTb3VyY2UvV1RGL3d0Zi90ZXh0L1RleHRC cmVha0l0ZXJhdG9yLmNwcCB8IDMxICsrKysrKysrKysrKysrKy0tLS0tLS0tLS0tLS0tLQogMiBm aWxlcyBjaGFuZ2VkLCA0OCBpbnNlcnRpb25zKCspLCAxNSBkZWxldGlvbnMoLSkKCmRpZmYgLS1n aXQgYS9Tb3VyY2UvV1RGL0NoYW5nZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IDlj NGEyNDcuLmJjNTI3M2EgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XVEYvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMzcgQEAKIDIwMTYtMDktMTQgIEpGIEJhc3Rp ZW4gIDxqZmJhc3RpZW5AYXBwbGUuY29tPgogCisgICAgICAgIFRleHRCcmVha0l0ZXJhdG9yOiB1 bmNvbnZvbHV0ZSBjaGFyYWN0ZXIgYnJlYWsgY2FjaGUKKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE2MjAwMQorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIFNpbXBsaWZ5IHRoZSBvbmUtZWxlbWVudCBjYWNoZS4K KworICAgICAgICBBZGRlZCBoZXJlIChmaXhlZCBhIGJpdCBhZnRlcik6IGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL2F0dGFjaG1lbnQuY2dpP2lkPTE0NDExOCZhY3Rpb249cHJldHR5cGF0Y2gKKyAg ICAgICAgVXBkYXRlZCB0byBuZXZlciB1c2UgYSBsb2NrLCBhbmQgYWx3YXlzIHVzZSB3ZWFrIGNt cHhjaGcgaGVyZTogaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvYXR0YWNobWVudC5jZ2k/aWQ9MjYx NzE5JmFjdGlvbj1wcmV0dHlwYXRjaAorCisgICAgICAgIEl0J3MganVzdCB0cnlpbmcgdG8gcmVk dWNlIHRoZSBudW1iZXIgb2YgdGltZXMgaXQgY2FsbHMgaW50byBJQ1UKKyAgICAgICAgdG8gaW5p dGlhbGl6ZSBhIFVCUktfQ0hBUkFDVEVSLiBUaGUgaW1wbGVtZW50YXRpb24ga2VlcHMgYQorICAg ICAgICBvbmUtZWxlbWVudCBjYWNoZSBvZiB0aGUgbGF0ZXN0IHVzZWQgb25lLCB3aGljaCB0aHJl YWRzIGNhbgorICAgICAgICBvcHRpbWlzdGljYWxseSBncmFiLiBGYWlsdXJlIGlzIGZpbmUgKGp1 c3QgY3JlYXRlIGEgbmV3IG9uZSksIHNhbWUKKyAgICAgICAgZm9yIGZhaWx1cmUgdG8gY2FjaGUg KGp1c3QgZGVzdHJveSBpdCksIGJ1dCB0aGUgbG9naWMgaXMgb2RkIGFuZAorICAgICAgICB5b3Ug dGVjaG5pY2FsbHkgbmVlZCByZWxlYXNlIC8gYWNxdWlyZSBzZW1hbnRpY3MgYmVjYXVzZSB0aGUK KyAgICAgICAgVUJSS19DSEFSQUNURVIgY3JlYXRpb24ncyBzdG9yZSBuZWVkIHRvIGJlIHZpc2li bGUgb24gYWNxdWlzaXRpb24KKyAgICAgICAgKGJ1dCByZWFsaXN0aWNhbGx5IGl0IHdhcyBjcmVh dGVkIGFuZCB0aGVuIHVzZWQgYW5kICp0aGVuKiBjYWNoZWQsCisgICAgICAgIHNvIGl0J3MgcHJv YmFibHkgYmVlbiBsb25nIGFnbyBlbm91Z2ggdGhhdCByZWFkIHJlb3JkZXJzIG5ldmVyCisgICAg ICAgIG1hbmlmZXN0ZWQpLgorCisgICAgICAgIFVzaW5nIGV4Y2hhbmdlIGRpcmVjdGx5IGFjaGll dmVzIHRoaXMgd2l0aG91dCB0aGUgaGVhZGFjaGUuCisKKyAgICAgICAgKiB3dGYvdGV4dC9UZXh0 QnJlYWtJdGVyYXRvci5jcHA6CisgICAgICAgIChXVEY6OmdldE5vblNoYXJlZENoYXJhY3RlckJy ZWFrSXRlcmF0b3IpOgorICAgICAgICAoV1RGOjpjYWNoZU5vblNoYXJlZENoYXJhY3RlckJyZWFr SXRlcmF0b3IpOgorICAgICAgICAoV1RGOjpOb25TaGFyZWRDaGFyYWN0ZXJCcmVha0l0ZXJhdG9y OjpOb25TaGFyZWRDaGFyYWN0ZXJCcmVha0l0ZXJhdG9yKToKKyAgICAgICAgKFdURjo6Tm9uU2hh cmVkQ2hhcmFjdGVyQnJlYWtJdGVyYXRvcjo6fk5vblNoYXJlZENoYXJhY3RlckJyZWFrSXRlcmF0 b3IpOgorICAgICAgICAoV1RGOjpjb21wYXJlQW5kU3dhcE5vblNoYXJlZENoYXJhY3RlckJyZWFr SXRlcmF0b3IpOiBEZWxldGVkLgorCisyMDE2LTA5LTE0ICBKRiBCYXN0aWVuICA8amZiYXN0aWVu QGFwcGxlLmNvbT4KKwogICAgICAgICBBdG9taWNzIG9uIEFSTSBkb24ndCByZXF1aXJlIGZ1bGwt c3lzdGVtIGZlbmNpbmcsIGFuZCBvdGhlciBtaW51dGlhZQogICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTYxOTI4CiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9X VEYvd3RmL3RleHQvVGV4dEJyZWFrSXRlcmF0b3IuY3BwIGIvU291cmNlL1dURi93dGYvdGV4dC9U ZXh0QnJlYWtJdGVyYXRvci5jcHAKaW5kZXggNjEyZDA5YS4uZmM3MGE3NyAxMDA2NDQKLS0tIGEv U291cmNlL1dURi93dGYvdGV4dC9UZXh0QnJlYWtJdGVyYXRvci5jcHAKKysrIGIvU291cmNlL1dU Ri93dGYvdGV4dC9UZXh0QnJlYWtJdGVyYXRvci5jcHAKQEAgLTI1LDggKzI1LDggQEAKICNpbmNs dWRlICJMaW5lQnJlYWtJdGVyYXRvclBvb2xJQ1UuaCIKICNpbmNsdWRlICJVVGV4dFByb3ZpZGVy TGF0aW4xLmgiCiAjaW5jbHVkZSAiVVRleHRQcm92aWRlclVURjE2LmgiCisjaW5jbHVkZSA8YXRv bWljPgogI2luY2x1ZGUgPG11dGV4PgotI2luY2x1ZGUgPHd0Zi9BdG9taWNzLmg+CiAjaW5jbHVk ZSA8d3RmL3RleHQvU3RyaW5nVmlldy5oPgogCiAvLyBGSVhNRTogVGhpcyBuZWVkcyBhIGJldHRl ciBuYW1lCkBAIC04MTMsMzAgKzgxMywzMSBAQCB2b2lkIGNsb3NlTGluZUJyZWFrSXRlcmF0b3Io VGV4dEJyZWFrSXRlcmF0b3IqJiBpdGVyYXRvcikKICAgICBpdGVyYXRvciA9IG51bGxwdHI7CiB9 CiAKLXN0YXRpYyBUZXh0QnJlYWtJdGVyYXRvciogbm9uU2hhcmVkQ2hhcmFjdGVyQnJlYWtJdGVy YXRvcjsKK3N0YXRpYyBzdGQ6OmF0b21pYzxUZXh0QnJlYWtJdGVyYXRvcio+IG5vblNoYXJlZENo YXJhY3RlckJyZWFrSXRlcmF0b3IgPSBBVE9NSUNfVkFSX0lOSVQobnVsbHB0cik7CiAKLXN0YXRp YyBpbmxpbmUgYm9vbCBjb21wYXJlQW5kU3dhcE5vblNoYXJlZENoYXJhY3RlckJyZWFrSXRlcmF0 b3IoVGV4dEJyZWFrSXRlcmF0b3IqIGV4cGVjdGVkLCBUZXh0QnJlYWtJdGVyYXRvciogbmV3VmFs dWUpCitzdGF0aWMgaW5saW5lIFRleHRCcmVha0l0ZXJhdG9yKiBnZXROb25TaGFyZWRDaGFyYWN0 ZXJCcmVha0l0ZXJhdG9yKCkKIHsKLSAgICByZXR1cm4gV1RGOjp3ZWFrQ29tcGFyZUFuZFN3YXAo Jm5vblNoYXJlZENoYXJhY3RlckJyZWFrSXRlcmF0b3IsIGV4cGVjdGVkLCBuZXdWYWx1ZSk7Cisg ICAgaWYgKGF1dG8gKnJlcyA9IG5vblNoYXJlZENoYXJhY3RlckJyZWFrSXRlcmF0b3IuZXhjaGFu Z2UobnVsbHB0ciwgc3RkOjptZW1vcnlfb3JkZXJfYWNxdWlyZSkpCisgICAgICAgIHJldHVybiBy ZXM7CisgICAgcmV0dXJuIGluaXRpYWxpemVJdGVyYXRvcihVQlJLX0NIQVJBQ1RFUik7CiB9CiAK LU5vblNoYXJlZENoYXJhY3RlckJyZWFrSXRlcmF0b3I6Ok5vblNoYXJlZENoYXJhY3RlckJyZWFr SXRlcmF0b3IoU3RyaW5nVmlldyBzdHJpbmcpCitzdGF0aWMgaW5saW5lIHZvaWQgY2FjaGVOb25T aGFyZWRDaGFyYWN0ZXJCcmVha0l0ZXJhdG9yKFRleHRCcmVha0l0ZXJhdG9yKiBjYWNoZU1lKQog ewotICAgIG1faXRlcmF0b3IgPSBub25TaGFyZWRDaGFyYWN0ZXJCcmVha0l0ZXJhdG9yOwotCi0g ICAgYm9vbCBjcmVhdGVkSXRlcmF0b3IgPSBtX2l0ZXJhdG9yICYmIGNvbXBhcmVBbmRTd2FwTm9u U2hhcmVkQ2hhcmFjdGVyQnJlYWtJdGVyYXRvcihtX2l0ZXJhdG9yLCBudWxscHRyKTsKLSAgICBp ZiAoIWNyZWF0ZWRJdGVyYXRvcikKLSAgICAgICAgbV9pdGVyYXRvciA9IGluaXRpYWxpemVJdGVy YXRvcihVQlJLX0NIQVJBQ1RFUik7Ci0gICAgaWYgKCFtX2l0ZXJhdG9yKQotICAgICAgICByZXR1 cm47CisgICAgaWYgKGF1dG8gKm9sZCA9IG5vblNoYXJlZENoYXJhY3RlckJyZWFrSXRlcmF0b3Iu ZXhjaGFuZ2UoY2FjaGVNZSwgc3RkOjptZW1vcnlfb3JkZXJfcmVsZWFzZSkpCisgICAgICAgIHVi cmtfY2xvc2UocmVpbnRlcnByZXRfY2FzdDxVQnJlYWtJdGVyYXRvcio+KG9sZCkpOworfQogCi0g ICAgbV9pdGVyYXRvciA9IHNldFRleHRGb3JJdGVyYXRvcigqbV9pdGVyYXRvciwgc3RyaW5nKTsK K05vblNoYXJlZENoYXJhY3RlckJyZWFrSXRlcmF0b3I6Ok5vblNoYXJlZENoYXJhY3RlckJyZWFr SXRlcmF0b3IoU3RyaW5nVmlldyBzdHJpbmcpCit7CisgICAgaWYgKChtX2l0ZXJhdG9yID0gZ2V0 Tm9uU2hhcmVkQ2hhcmFjdGVyQnJlYWtJdGVyYXRvcigpKSkKKyAgICAgICAgbV9pdGVyYXRvciA9 IHNldFRleHRGb3JJdGVyYXRvcigqbV9pdGVyYXRvciwgc3RyaW5nKTsKIH0KIAogTm9uU2hhcmVk Q2hhcmFjdGVyQnJlYWtJdGVyYXRvcjo6fk5vblNoYXJlZENoYXJhY3RlckJyZWFrSXRlcmF0b3Io KQogewotICAgIGlmICghY29tcGFyZUFuZFN3YXBOb25TaGFyZWRDaGFyYWN0ZXJCcmVha0l0ZXJh dG9yKDAsIG1faXRlcmF0b3IpKQotICAgICAgICB1YnJrX2Nsb3NlKHJlaW50ZXJwcmV0X2Nhc3Q8 VUJyZWFrSXRlcmF0b3IqPihtX2l0ZXJhdG9yKSk7CisgICAgaWYgKG1faXRlcmF0b3IpCisgICAg ICAgIGNhY2hlTm9uU2hhcmVkQ2hhcmFjdGVyQnJlYWtJdGVyYXRvcihtX2l0ZXJhdG9yKTsKIH0K IAogTm9uU2hhcmVkQ2hhcmFjdGVyQnJlYWtJdGVyYXRvcjo6Tm9uU2hhcmVkQ2hhcmFjdGVyQnJl YWtJdGVyYXRvcihOb25TaGFyZWRDaGFyYWN0ZXJCcmVha0l0ZXJhdG9yJiYgb3RoZXIpCi0tIAoy LjkuMwoK