152662 2016-01-03 01:25:45 -0800 StringBuilder often creates two StringImpls. 2016-01-03 12:37:52 -0800 1 1 1 Unclassified WebKit Web Template Framework WebKit Nightly Build Unspecified Unspecified REOPENED Performance P2 Normal --- 1 kling kling benjamin cdumez cmarcelo commit-queue kling simon.fraser oldest_to_newest 1152089 0 kling 2016-01-03 01:25:45 -0800 Sometimes StringBuilder will keep the internal buffer used during building, and the returned "built" StringImpl will be a substring that retains the buffer. This way we keep two StringImpls around instead of one. 1152090 1 268134 kling 2016-01-03 01:29:15 -0800 Created attachment 268134 Proposed patch Here's one way of making this better: adjusting the length field of the buffer and then returning it as the final string. This way we keep the slightly oversized buffer allocation, but don't allocate an extra StringImpl. 1152103 2 268134 commit-queue 2016-01-03 08:12:42 -0800 Comment on attachment 268134 Proposed patch Clearing flags on attachment: 268134 Committed r194510: <http://trac.webkit.org/changeset/194510> 1152104 3 commit-queue 2016-01-03 08:12:47 -0800 All reviewed patches have been landed. Closing bug. 1152115 4 simon.fraser 2016-01-03 09:36:00 -0800 This broke the windows build: C:\cygwin\home\buildbot\slave\win-debug\build\Source\WTF\wtf\text\StringBuilder.cpp(63): error C2248: 'WTF::StringImpl::m_length': cannot access private member declared in class 'WTF::StringImpl' [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\WTF\wtf\WTF.vcxproj] 1152116 5 simon.fraser 2016-01-03 09:37:08 -0800 It also broke two tests: Tests that failed: StringBuilderTest.Append StringBuilderTest.ToStringPreserveCapacity 1152156 6 kling 2016-01-03 12:37:52 -0800 Reverted r194510 for reason: Broke 2 API tests, no time to investigate right now Committed r194519: <http://trac.webkit.org/changeset/194519> 268134 2016-01-03 01:29:15 -0800 2016-01-03 08:12:42 -0800 Proposed patch bug-152662.diff text/plain 2565 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nIGIvU291cmNlL1dURi9DaGFuZ2VMb2cK aW5kZXggYjhmOTg5OS4uYjUxNTBkMCAxMDA2NDQKLS0tIGEvU291cmNlL1dURi9DaGFuZ2VMb2cK KysrIGIvU291cmNlL1dURi9DaGFuZ2VMb2cKQEAgLTEsMyArMSwyNSBAQAorMjAxNi0wMS0wMyAg QW5kcmVhcyBLbGluZyAgPGFrbGluZ0BhcHBsZS5jb20+CisKKyAgICAgICAgU3RyaW5nQnVpbGRl ciBvZnRlbiBjcmVhdGVzIHR3byBTdHJpbmdJbXBscy4KKyAgICAgICAgPGh0dHBzOi8vd2Via2l0 Lm9yZy9iLzE1MjY2Mj4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICBJZiBTdHJpbmdCdWlsZGVyIGRpZG4ndCBtYW5hZ2UgdG8gZ3Vlc3MgdGhlIGV4YWN0 IGZpbmFsIGxlbmd0aCBvZiB0aGUKKyAgICAgICAgc3RyaW5nIGJlaW5nIGJ1aWx0LCBpdCB3b3Vs ZCBzdGlsbCBrZWVwIHRoZSBidWZmZXIgYXJvdW5kLCBhbmQgdGhlIGZpbmFsCisgICAgICAgIGJ1 aWxkIHByb2Nlc3Mgd291bGQgdGhlbiByZXR1cm4gYSBzdWJzdHJpbmcgaW50byB0aGF0IGJ1ZmZl ci4KKworICAgICAgICBUaGlzIGVmZmVjdGl2ZWx5IHlpZWxkZWQgdHdvIFN0cmluZ0ltcGxzIGlu c3RlYWQgb2Ygb25lLCB3aXRoIHRoZSBzdWJzdHJpbmcKKyAgICAgICAgcmV0YWluaW5nIHRoZSBs b25nZXIgYnVmZmVyIFN0cmluZ0ltcGwuCisKKyAgICAgICAgVGhpcyBwYXRjaCBpbXByb3ZlcyB0 aGUgc2l0dWF0aW9uIGJ5IGhhdmluZyBTdHJpbmdJbXBsIGNyb3AgdGhlIGJ1ZmZlcidzCisgICAg ICAgIG1fbGVuZ3RoIGZpZWxkIHRvIHRoZSBmaW5hbCBidWlsdCBzdHJpbmcgbGVuZ3RoLiBUaGF0 IHdheSB3ZSBuZXZlciBoYXZlIHRvCisgICAgICAgIHJldHVybiBhIHN1YnN0cmluZy4KKworICAg ICAgICAqIHd0Zi90ZXh0L1N0cmluZ0J1aWxkZXIuY3BwOgorICAgICAgICAoV1RGOjpTdHJpbmdC dWlsZGVyOjpyZWlmeVN0cmluZyk6CisgICAgICAgICogd3RmL3RleHQvU3RyaW5nSW1wbC5oOgor CiAyMDE1LTEyLTMxICBBbmR5IEVzdGVzICA8YWVzdGVzQGFwcGxlLmNvbT4KIAogICAgICAgICBS ZXBsYWNlIFdURjo6bW92ZSB3aXRoIFdURk1vdmUKZGlmZiAtLWdpdCBhL1NvdXJjZS9XVEYvd3Rm L3RleHQvU3RyaW5nQnVpbGRlci5jcHAgYi9Tb3VyY2UvV1RGL3d0Zi90ZXh0L1N0cmluZ0J1aWxk ZXIuY3BwCmluZGV4IDgxYzNhODguLjI2MDM5ZWEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvd3Rm L3RleHQvU3RyaW5nQnVpbGRlci5jcHAKKysrIGIvU291cmNlL1dURi93dGYvdGV4dC9TdHJpbmdC dWlsZGVyLmNwcApAQCAtNTQsMTIgKzU0LDE0IEBAIHZvaWQgU3RyaW5nQnVpbGRlcjo6cmVpZnlT dHJpbmcoKSBjb25zdAogICAgICAgICByZXR1cm47CiAgICAgfQogCi0gICAgLy8gTXVzdCBiZSB2 YWxpZCBpbiB0aGUgYnVmZmVyLCB0YWtlIGEgc3Vic3RyaW5nICh1bmxlc3Mgc3RyaW5nIGZpbGxz IHRoZSBidWZmZXIpLgotICAgIEFTU0VSVChtX2J1ZmZlciAmJiBtX2xlbmd0aCA8PSBtX2J1ZmZl ci0+bGVuZ3RoKCkpOwotICAgIGlmIChtX2xlbmd0aCA9PSBtX2J1ZmZlci0+bGVuZ3RoKCkpCi0g ICAgICAgIG1fc3RyaW5nID0gbV9idWZmZXIuZ2V0KCk7Ci0gICAgZWxzZQotICAgICAgICBtX3N0 cmluZyA9IFN0cmluZ0ltcGw6OmNyZWF0ZVN1YnN0cmluZ1NoYXJpbmdJbXBsKG1fYnVmZmVyLCAw LCBtX2xlbmd0aCk7CisgICAgQVNTRVJUKG1fYnVmZmVyKTsKKyAgICBBU1NFUlQobV9sZW5ndGgg PD0gbV9idWZmZXItPmxlbmd0aCgpKTsKKworICAgIC8vICJDcm9wIiB0aGUgU3RyaW5nSW1wbCBi eSB1cGRhdGluZyBpdHMgbV9sZW5ndGggZmllbGQuIE5vdGUgdGhhdCB0aGlzIHdpbGwKKyAgICAv LyByZXN1bHQgaW4gc29tZSBTdHJpbmdJbXBsIG9iamVjdHMgYmVpbmcgYmFja2VkIGJ5IGEgc2xp Z2h0bHkgbGFyZ2VyIGFsbG9jYXRpb24KKyAgICAvLyB0aGFuIHRoZWlyIG1fbGVuZ3RoIGZpZWxk IGluZGljYXRlcy4KKyAgICBtX2J1ZmZlci0+bV9sZW5ndGggPSBtX2xlbmd0aDsKKyAgICBtX3N0 cmluZyA9IG1fYnVmZmVyLmdldCgpOwogfQogCiB2b2lkIFN0cmluZ0J1aWxkZXI6OnJlc2l6ZSh1 bnNpZ25lZCBuZXdTaXplKQpkaWZmIC0tZ2l0IGEvU291cmNlL1dURi93dGYvdGV4dC9TdHJpbmdJ bXBsLmggYi9Tb3VyY2UvV1RGL3d0Zi90ZXh0L1N0cmluZ0ltcGwuaAppbmRleCA2OTQ0MmM2Li4w ODczM2I0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV1RGL3d0Zi90ZXh0L1N0cmluZ0ltcGwuaAorKysg Yi9Tb3VyY2UvV1RGL3d0Zi90ZXh0L1N0cmluZ0ltcGwuaApAQCAtMTM1LDYgKzEzNSw3IEBAIGNs YXNzIFN0cmluZ0ltcGwgewogICAgIGZyaWVuZCBzdHJ1Y3QgV1RGOjpVQ2hhckJ1ZmZlclRyYW5z bGF0b3I7CiAgICAgZnJpZW5kIGNsYXNzIEpTQzo6TExJbnQ6OkRhdGE7CiAgICAgZnJpZW5kIGNs YXNzIEpTQzo6TExJbnRPZmZzZXRzRXh0cmFjdG9yOworICAgIGZyaWVuZCBjbGFzcyBXVEY6OlN0 cmluZ0J1aWxkZXI7CiAgICAgCiBwcml2YXRlOgogICAgIGVudW0gQnVmZmVyT3duZXJzaGlwIHsK