58063 2011-04-07 11:02:18 -0700 CTLine objects should outlive their CTRuns 2011-04-10 09:11:55 -0700 1 1 1 Unclassified WebKit Text 528+ (Nightly build) Mac OS X 10.6 RESOLVED FIXED InRadar P2 Normal --- 1 ned webkit-unassigned commit-queue mitz oldest_to_newest 381492 0 ned 2011-04-07 11:02:18 -0700 CoreText may need to perform some extra work if a CTLine object is released prior to its CTRuns, but this extra work can be avoided by either retaining CTLines longer than their CTRuns (in ComplexTextController), or by not retaining the CTRuns (in ComplexTextRun). 381493 1 ned 2011-04-07 11:04:40 -0700 The latter approach is less desirable since it would eliminate the possibility of using the CTRunGet*Ptr() accessors, so I'll work up some numbers for the former. 381843 2 88732 ned 2011-04-07 16:44:28 -0700 Created attachment 88732 Proposed changes. Numbers are a bit variable on my machine, but I'm seeing an improvement of around 2% for repeated optimizeLegibility test runs on Latin text—barely above the level of noise but certainly not a slowdown. 381862 3 88732 mitz 2011-04-07 17:07:14 -0700 Comment on attachment 88732 Proposed changes. If the CTLine stays alive, then maybe ComplexTextRun::m_coreTextRun no longer needs to be a RetainPtr? 381865 4 ned 2011-04-07 17:09:14 -0700 (In reply to comment #3) > (From update of attachment 88732 [details]) > If the CTLine stays alive, then maybe ComplexTextRun::m_coreTextRun no longer needs to be a RetainPtr? Good idea! Should it become a different smart pointer or will a bare CTRunRef suffice? 381867 5 mitz 2011-04-07 17:15:51 -0700 (In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 88732 [details] [details]) > > If the CTLine stays alive, then maybe ComplexTextRun::m_coreTextRun no longer needs to be a RetainPtr? > > Good idea! Should it become a different smart pointer or will a bare CTRunRef suffice? Bare CTRunRef. A comment right above the member variable definition explaining why it’s ok to use a bare CTRunRef would be good. 381877 6 ned 2011-04-07 17:31:18 -0700 (In reply to comment #5) > Bare CTRunRef. A comment right above the member variable definition explaining why it’s ok to use a bare CTRunRef would be good. Theoretically it could be removed entirely since it's only used in the constructor. 381878 7 mitz 2011-04-07 17:33:39 -0700 (In reply to comment #6) > (In reply to comment #5) > > Bare CTRunRef. A comment right above the member variable definition explaining why it’s ok to use a bare CTRunRef would be good. > > Theoretically it could be removed entirely since it's only used in the constructor. Let’s remove it in practice, then. 381888 8 88743 ned 2011-04-07 17:50:00 -0700 Created attachment 88743 Remove m_coreTextRun. 381991 9 88743 commit-queue 2011-04-07 21:38:56 -0700 Comment on attachment 88743 Remove m_coreTextRun. Clearing flags on attachment: 88743 Committed r83251: <http://trac.webkit.org/changeset/83251> 381992 10 commit-queue 2011-04-07 21:39:00 -0700 All reviewed patches have been landed. Closing bug. 382892 11 mitz 2011-04-10 09:11:55 -0700 <rdar://problem/9250245> 88732 2011-04-07 16:44:28 -0700 2011-04-07 17:50:00 -0700 Proposed changes. lines-vs-runs.txt text/plain 2132 ned SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDgzMjEyKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTEtMDQtMDcgIE5lZCBIb2xi cm9vayAgPG5ob2xicm9va0BhcHBsZS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgQ1RMaW5lIG9iamVjdHMgc2hvdWxkIG91dGxpdmUgdGhlaXIg Q1RSdW5zCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD01 ODA2MworCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvbWFjL0NvbXBsZXhUZXh0Q29udHJv bGxlci5oOiBBZGQgbV9jb3JlVGV4dExpbmVzLCB0byBiZSBkZXN0cm95ZWQgYWZ0ZXIgbV9jb21w bGV4VGV4dFJ1bnMuCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvbWFjL0NvbXBsZXhUZXh0 Q29udHJvbGxlckNvcmVUZXh0LmNwcDogQXBwZW5kIGxpbmUgdG8gbV9jb3JlVGV4dExpbmVzLgor ICAgICAgICAoV2ViQ29yZTo6Q29tcGxleFRleHRDb250cm9sbGVyOjpjb2xsZWN0Q29tcGxleFRl eHRSdW5zRm9yQ2hhcmFjdGVyc0NvcmVUZXh0KToKKwogMjAxMS0wNC0wNyAgU2VyZ2V5IEdsYXp1 bm92ICA8c2VyZy5nbGF6dW5vdkBnbWFpbC5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRGlt aXRyaSBHbGF6a292LgpJbmRleDogU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvbWFj L0NvbXBsZXhUZXh0Q29udHJvbGxlci5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3Bs YXRmb3JtL2dyYXBoaWNzL21hYy9Db21wbGV4VGV4dENvbnRyb2xsZXIuaAkocmV2aXNpb24gODI4 NzIpCisrKyBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9tYWMvQ29tcGxleFRleHRD b250cm9sbGVyLmgJKHdvcmtpbmcgY29weSkKQEAgLTE1OSw2ICsxNTksMTAgQEAgcHJpdmF0ZToK IAogICAgIFZlY3RvcjxVQ2hhciwgMjU2PiBtX3NtYWxsQ2Fwc0J1ZmZlcjsKIAorI2lmIFVTRShD T1JFX1RFWFQpCisgICAgLy8gTWFrZSBzdXJlIGxpbmVzIG91dGxpdmUgdGhlaXIgcnVucyBmb3Ig cGVyZm9ybWFuY2UgcmVhc29ucworICAgIFZlY3RvcjxSZXRhaW5QdHI8Q1RMaW5lUmVmPiA+IG1f Y29yZVRleHRMaW5lczsKKyNlbmRpZgogICAgIFZlY3RvcjxSZWZQdHI8Q29tcGxleFRleHRSdW4+ LCAxNj4gbV9jb21wbGV4VGV4dFJ1bnM7CiAgICAgVmVjdG9yPENHU2l6ZSwgMjU2PiBtX2FkanVz dGVkQWR2YW5jZXM7CiAgICAgVmVjdG9yPENHR2x5cGgsIDI1Nj4gbV9hZGp1c3RlZEdseXBoczsK SW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL21hYy9Db21wbGV4VGV4dENv bnRyb2xsZXJDb3JlVGV4dC5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGxhdGZv cm0vZ3JhcGhpY3MvbWFjL0NvbXBsZXhUZXh0Q29udHJvbGxlckNvcmVUZXh0LmNwcAkocmV2aXNp b24gODI4NzIpCisrKyBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9tYWMvQ29tcGxl eFRleHRDb250cm9sbGVyQ29yZVRleHQuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xNTksNiArMTU5 LDggQEAgdm9pZCBDb21wbGV4VGV4dENvbnRyb2xsZXI6OmNvbGxlY3RDb21wbAogICAgICAgICBs aW5lLmFkb3B0Q0Yod2tDcmVhdGVDVExpbmVXaXRoVW5pQ2hhclByb3ZpZGVyKCZwcm92aWRlU3Ry aW5nQW5kQXR0cmlidXRlcywgMCwgJmluZm8pKTsKICAgICB9CiAKKyAgICBtX2NvcmVUZXh0TGlu ZXMuYXBwZW5kKGxpbmUuZ2V0KCkpOworCiAgICAgQ0ZBcnJheVJlZiBydW5BcnJheSA9IENUTGlu ZUdldEdseXBoUnVucyhsaW5lLmdldCgpKTsKIAogICAgIENGSW5kZXggcnVuQ291bnQgPSBDRkFy cmF5R2V0Q291bnQocnVuQXJyYXkpOwo= 88743 2011-04-07 17:50:00 -0700 2011-04-07 21:38:56 -0700 Remove m_coreTextRun. lines-vs-no-runs.txt text/plain 4485 ned SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDgzMjMwKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMDQtMDcgIE5lZCBIb2xi cm9vayAgPG5ob2xicm9va0BhcHBsZS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgQ1RMaW5lIG9iamVjdHMgc2hvdWxkIG91dGxpdmUgdGhlaXIg Q1RSdW5zCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD01 ODA2MworCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvbWFjL0NvbXBsZXhUZXh0Q29udHJv bGxlci5oOiBBZGQgbV9jb3JlVGV4dExpbmVzLCB0byBiZSBkZXN0cm95ZWQgYWZ0ZXIgbV9jb21w bGV4VGV4dFJ1bnMuCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvbWFjL0NvbXBsZXhUZXh0 Q29udHJvbGxlckNvcmVUZXh0LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkNvbXBsZXhUZXh0Q29u dHJvbGxlcjo6Q29tcGxleFRleHRSdW46OkNvbXBsZXhUZXh0UnVuKTogTW9vdCBtX2NvcmVUZXh0 UnVuLgorICAgICAgICAoV2ViQ29yZTo6Q29tcGxleFRleHRDb250cm9sbGVyOjpjb2xsZWN0Q29t cGxleFRleHRSdW5zRm9yQ2hhcmFjdGVyc0NvcmVUZXh0KTogQXBwZW5kIGxpbmUgdG8gbV9jb3Jl VGV4dExpbmVzLgorCiAyMDExLTA0LTA3ICBEYW4gQmVybnN0ZWluICA8bWl0ekBhcHBsZS5jb20+ CiAKICAgICAgICAgQnVpbGQgZml4LgpJbmRleDogU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3Jh cGhpY3MvbWFjL0NvbXBsZXhUZXh0Q29udHJvbGxlci5oCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9X ZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL21hYy9Db21wbGV4VGV4dENvbnRyb2xsZXIuaAkocmV2 aXNpb24gODI4NzIpCisrKyBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9tYWMvQ29t cGxleFRleHRDb250cm9sbGVyLmgJKHdvcmtpbmcgY29weSkKQEAgLTExNiw5ICsxMTYsNiBAQCBw cml2YXRlOgogICAgICAgICBzdGF0aWMgT1NTdGF0dXMgb3ZlcnJpZGVMYXlvdXRPcGVyYXRpb24o QVRTVUxheW91dE9wZXJhdGlvblNlbGVjdG9yLCBBVFNVTGluZVJlZiwgVVJlZkNvbiwgdm9pZCos IEFUU1VMYXlvdXRPcGVyYXRpb25DYWxsYmFja1N0YXR1cyopOwogI2VuZGlmCiAKLSNpZiBVU0Uo Q09SRV9URVhUKQotICAgICAgICBSZXRhaW5QdHI8Q1RSdW5SZWY+IG1fY29yZVRleHRSdW47Ci0j ZW5kaWYKICAgICAgICAgdW5zaWduZWQgbV9nbHlwaENvdW50OwogICAgICAgICBjb25zdCBTaW1w bGVGb250RGF0YSogbV9mb250RGF0YTsKICAgICAgICAgY29uc3QgVUNoYXIqIG1fY2hhcmFjdGVy czsKQEAgLTE1OSw2ICsxNTYsMTAgQEAgcHJpdmF0ZToKIAogICAgIFZlY3RvcjxVQ2hhciwgMjU2 PiBtX3NtYWxsQ2Fwc0J1ZmZlcjsKIAorI2lmIFVTRShDT1JFX1RFWFQpCisgICAgLy8gUmV0YWlu IGxpbmVzIHJhdGhlciB0aGFuIHRoZWlyIHJ1bnMgZm9yIGJldHRlciBwZXJmb3JtYW5jZS4KKyAg ICBWZWN0b3I8UmV0YWluUHRyPENUTGluZVJlZj4gPiBtX2NvcmVUZXh0TGluZXM7CisjZW5kaWYK ICAgICBWZWN0b3I8UmVmUHRyPENvbXBsZXhUZXh0UnVuPiwgMTY+IG1fY29tcGxleFRleHRSdW5z OwogICAgIFZlY3RvcjxDR1NpemUsIDI1Nj4gbV9hZGp1c3RlZEFkdmFuY2VzOwogICAgIFZlY3Rv cjxDR0dseXBoLCAyNTY+IG1fYWRqdXN0ZWRHbHlwaHM7CkluZGV4OiBTb3VyY2UvV2ViQ29yZS9w bGF0Zm9ybS9ncmFwaGljcy9tYWMvQ29tcGxleFRleHRDb250cm9sbGVyQ29yZVRleHQuY3BwCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL21hYy9Db21wbGV4 VGV4dENvbnRyb2xsZXJDb3JlVGV4dC5jcHAJKHJldmlzaW9uIDgyODcyKQorKysgU291cmNlL1dl YkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvbWFjL0NvbXBsZXhUZXh0Q29udHJvbGxlckNvcmVUZXh0 LmNwcAkod29ya2luZyBjb3B5KQpAQCAtNDMsMzMgKzQzLDMyIEBAIGV4dGVybiBjb25zdCBDRlN0 cmluZ1JlZiBrQ1RUeXBlc2V0dGVyT3AKIG5hbWVzcGFjZSBXZWJDb3JlIHsKIAogQ29tcGxleFRl eHRDb250cm9sbGVyOjpDb21wbGV4VGV4dFJ1bjo6Q29tcGxleFRleHRSdW4oQ1RSdW5SZWYgY3RS dW4sIGNvbnN0IFNpbXBsZUZvbnREYXRhKiBmb250RGF0YSwgY29uc3QgVUNoYXIqIGNoYXJhY3Rl cnMsIHVuc2lnbmVkIHN0cmluZ0xvY2F0aW9uLCBzaXplX3Qgc3RyaW5nTGVuZ3RoLCBDRlJhbmdl IHJ1blJhbmdlKQotICAgIDogbV9jb3JlVGV4dFJ1bihjdFJ1bikKLSAgICAsIG1fZm9udERhdGEo Zm9udERhdGEpCisgICAgOiBtX2ZvbnREYXRhKGZvbnREYXRhKQogICAgICwgbV9jaGFyYWN0ZXJz KGNoYXJhY3RlcnMpCiAgICAgLCBtX3N0cmluZ0xvY2F0aW9uKHN0cmluZ0xvY2F0aW9uKQogICAg ICwgbV9zdHJpbmdMZW5ndGgoc3RyaW5nTGVuZ3RoKQogICAgICwgbV9pbmRleEVuZChydW5SYW5n ZS5sb2NhdGlvbiArIHJ1blJhbmdlLmxlbmd0aCkKICAgICAsIG1faXNNb25vdG9uaWModHJ1ZSkK IHsKLSAgICBtX2dseXBoQ291bnQgPSBDVFJ1bkdldEdseXBoQ291bnQobV9jb3JlVGV4dFJ1bi5n ZXQoKSk7Ci0gICAgbV9jb3JlVGV4dEluZGljZXMgPSBDVFJ1bkdldFN0cmluZ0luZGljZXNQdHIo bV9jb3JlVGV4dFJ1bi5nZXQoKSk7CisgICAgbV9nbHlwaENvdW50ID0gQ1RSdW5HZXRHbHlwaENv dW50KGN0UnVuKTsKKyAgICBtX2NvcmVUZXh0SW5kaWNlcyA9IENUUnVuR2V0U3RyaW5nSW5kaWNl c1B0cihjdFJ1bik7CiAgICAgaWYgKCFtX2NvcmVUZXh0SW5kaWNlcykgewogICAgICAgICBtX2Nv cmVUZXh0SW5kaWNlc1ZlY3Rvci5ncm93KG1fZ2x5cGhDb3VudCk7Ci0gICAgICAgIENUUnVuR2V0 U3RyaW5nSW5kaWNlcyhtX2NvcmVUZXh0UnVuLmdldCgpLCBDRlJhbmdlTWFrZSgwLCAwKSwgbV9j b3JlVGV4dEluZGljZXNWZWN0b3IuZGF0YSgpKTsKKyAgICAgICAgQ1RSdW5HZXRTdHJpbmdJbmRp Y2VzKGN0UnVuLCBDRlJhbmdlTWFrZSgwLCAwKSwgbV9jb3JlVGV4dEluZGljZXNWZWN0b3IuZGF0 YSgpKTsKICAgICAgICAgbV9jb3JlVGV4dEluZGljZXMgPSBtX2NvcmVUZXh0SW5kaWNlc1ZlY3Rv ci5kYXRhKCk7CiAgICAgfQogCi0gICAgbV9nbHlwaHMgPSBDVFJ1bkdldEdseXBoc1B0cihtX2Nv cmVUZXh0UnVuLmdldCgpKTsKKyAgICBtX2dseXBocyA9IENUUnVuR2V0R2x5cGhzUHRyKGN0UnVu KTsKICAgICBpZiAoIW1fZ2x5cGhzKSB7CiAgICAgICAgIG1fZ2x5cGhzVmVjdG9yLmdyb3cobV9n bHlwaENvdW50KTsKLSAgICAgICAgQ1RSdW5HZXRHbHlwaHMobV9jb3JlVGV4dFJ1bi5nZXQoKSwg Q0ZSYW5nZU1ha2UoMCwgMCksIG1fZ2x5cGhzVmVjdG9yLmRhdGEoKSk7CisgICAgICAgIENUUnVu R2V0R2x5cGhzKGN0UnVuLCBDRlJhbmdlTWFrZSgwLCAwKSwgbV9nbHlwaHNWZWN0b3IuZGF0YSgp KTsKICAgICAgICAgbV9nbHlwaHMgPSBtX2dseXBoc1ZlY3Rvci5kYXRhKCk7CiAgICAgfQogCi0g ICAgbV9hZHZhbmNlcyA9IENUUnVuR2V0QWR2YW5jZXNQdHIobV9jb3JlVGV4dFJ1bi5nZXQoKSk7 CisgICAgbV9hZHZhbmNlcyA9IENUUnVuR2V0QWR2YW5jZXNQdHIoY3RSdW4pOwogICAgIGlmICgh bV9hZHZhbmNlcykgewogICAgICAgICBtX2FkdmFuY2VzVmVjdG9yLmdyb3cobV9nbHlwaENvdW50 KTsKLSAgICAgICAgQ1RSdW5HZXRBZHZhbmNlcyhtX2NvcmVUZXh0UnVuLmdldCgpLCBDRlJhbmdl TWFrZSgwLCAwKSwgbV9hZHZhbmNlc1ZlY3Rvci5kYXRhKCkpOworICAgICAgICBDVFJ1bkdldEFk dmFuY2VzKGN0UnVuLCBDRlJhbmdlTWFrZSgwLCAwKSwgbV9hZHZhbmNlc1ZlY3Rvci5kYXRhKCkp OwogICAgICAgICBtX2FkdmFuY2VzID0gbV9hZHZhbmNlc1ZlY3Rvci5kYXRhKCk7CiAgICAgfQog fQpAQCAtMTU5LDYgKzE1OCw4IEBAIHZvaWQgQ29tcGxleFRleHRDb250cm9sbGVyOjpjb2xsZWN0 Q29tcGwKICAgICAgICAgbGluZS5hZG9wdENGKHdrQ3JlYXRlQ1RMaW5lV2l0aFVuaUNoYXJQcm92 aWRlcigmcHJvdmlkZVN0cmluZ0FuZEF0dHJpYnV0ZXMsIDAsICZpbmZvKSk7CiAgICAgfQogCisg ICAgbV9jb3JlVGV4dExpbmVzLmFwcGVuZChsaW5lLmdldCgpKTsKKwogICAgIENGQXJyYXlSZWYg cnVuQXJyYXkgPSBDVExpbmVHZXRHbHlwaFJ1bnMobGluZS5nZXQoKSk7CiAKICAgICBDRkluZGV4 IHJ1bkNvdW50ID0gQ0ZBcnJheUdldENvdW50KHJ1bkFycmF5KTsK