101409 2012-11-06 16:29:58 -0800 Make Document::renderer faster by using the cached ptr for RenderView 2012-11-06 19:32:02 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 esprehn esprehn eric webkit.review.bot oldest_to_newest 760190 0 esprehn 2012-11-06 16:29:58 -0800 Make Document::renderer faster by using the cached ptr for RenderView 760201 1 172677 esprehn 2012-11-06 16:41:47 -0800 Created attachment 172677 Patch 760214 2 esprehn 2012-11-06 16:59:23 -0800 In pondering this patch I wonder if it's better to just change callers to use document()->renderView() and make Document::renderer private? That avoids all this casting stuff and the out of line method, but it means changing many more files. As an advantage it would also stop people from doing the silliness that's in several places where they think document()->renderer() != document()->renderer()->view(), the only downside is that it means including RenderView.h whenever you want to access the renderer on the document. Any opinion on this Eric? 760225 3 eric 2012-11-06 17:12:57 -0800 So this is just reverting your previous change? 760248 4 esprehn 2012-11-06 17:47:36 -0800 (In reply to comment #3) > So this is just reverting your previous change? Only partly, my previous change made renderView() return a pointer stored in Document skipping the RareData union stuff in Node::renderer(). so without this patch: Document::renderer() => Still goes through Node::renderer() and checks the hasRareData() and stuff. Document::renderView() => Direct ptr access without the conditional With this patch: Document::renderer() => Returns m_renderer and skips the union hasRareData logic. Document::renderView() => Also kips the logic, but needs the cast and to be in RenderView.h My other alternative is: Document::renderer() is private and just calls the "slow" Node::renderer() Change all callers of document()->renderer() to do document()->renderView() That last option means changing tons of files, but is more explicit at least. 760261 5 172677 eric 2012-11-06 18:07:29 -0800 Comment on attachment 172677 Patch OK. 760308 6 172677 webkit.review.bot 2012-11-06 19:31:59 -0800 Comment on attachment 172677 Patch Clearing flags on attachment: 172677 Committed r133711: <http://trac.webkit.org/changeset/133711> 760309 7 webkit.review.bot 2012-11-06 19:32:02 -0800 All reviewed patches have been landed. Closing bug. 172677 2012-11-06 16:41:47 -0800 2012-11-06 19:31:59 -0800 Patch bug-101409-20121106163959.patch text/plain 4058 esprehn U3VidmVyc2lvbiBSZXZpc2lvbjogMTMzNjkyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNmE5YmQ2ZjM4OGNmMWUz NmFhMGMzM2NhM2Y1NzYzNWMzMTJlODZkMC4uMTE2MjE0YzIzYTJkMDllMWI5YWJlZWZjMzg3NWNk Y2VmNjZmZTk4ZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIzIEBACisyMDEyLTExLTA2ICBFbGxp b3R0IFNwcmVobiAgPGVzcHJlaG5AY2hyb21pdW0ub3JnPgorCisgICAgICAgIE1ha2UgRG9jdW1l bnQ6OnJlbmRlcmVyIGZhc3RlciBieSB1c2luZyB0aGUgY2FjaGVkIHB0ciBmb3IgUmVuZGVyVmll dworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTAxNDA5 CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSW4gQnVn IDEwMTI3NyBJIG1hZGUgRG9jdW1lbnQ6OnJlbmRlclZpZXcoKSBqdXN0IHJldHVybiBtX3JlbmRl clZpZXcgaW5zdGVhZAorICAgICAgICBvZiBjYXN0aW5nIHRoZSByZXN1bHQgb2YgcmVuZGVyZXIo KS4gV2hpbGUgdGhpcyBtYWRlIHJlbmRlclZpZXcoKSBjaGVhcGVyIGl0CisgICAgICAgIG1ha2Vz IG1vcmUgc2Vuc2UgdG8ganVzdCBtYWtlIHJlbmRlcmVyKCkgZmFzdGVyIGZvciBEb2N1bWVudCBw b2ludGVycyBhbmQKKyAgICAgICAgZ28gYmFjayB0byB0aGUgb3V0IG9mIGxpbmUgbWV0aG9kIGlu IFJlbmRlclZpZXcuaCBiZWNhdXNlIGxvdHMgb2YgcGxhY2VzIGluCisgICAgICAgIHRoZSBjb2Rl IGRvIGRvY3VtZW50KCktPnJlbmRlcmVyKCkuCisKKyAgICAgICAgTm8gbmV3IHRlc3RzLCB0aGlz IGlzIGp1c3QgYSByZWZhY3Rvci4KKworICAgICAgICAqIGRvbS9Eb2N1bWVudC5oOgorICAgICAg ICAoV2ViQ29yZTo6RG9jdW1lbnQ6OnJlbmRlcmVyKToKKyAgICAgICAgKiByZW5kZXJpbmcvUmVu ZGVyVmlldy5oOgorICAgICAgICAoV2ViQ29yZTo6RG9jdW1lbnQ6OnJlbmRlclZpZXcpOgorCiAy MDEyLTExLTA2ICBIdWFuZyBEb25nc3VuZyAgPGx1eHRlbGxhQGNvbXBhbnkxMDAubmV0PgogCiAg ICAgICAgIEJ1aWxkIGZpeC4gcjEzMzYwMSBicm9rZSB0aGUgV2luZG93cyBidWlsZC4KZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL2RvbS9Eb2N1bWVudC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9k b20vRG9jdW1lbnQuY3BwCmluZGV4IDU0YThmMWE5NGY4NjkwNTEwNTI5MGU5YzA3NjVmZjNkZDdh ZDQxNDEuLjMxZmRiYWVkNDg5OGQyZTBjZDE1OWVhYzg2YjU4N2E1ZjY4ZTliMjggMTAwNjQ0Ci0t LSBhL1NvdXJjZS9XZWJDb3JlL2RvbS9Eb2N1bWVudC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUv ZG9tL0RvY3VtZW50LmNwcApAQCAtNDc4LDcgKzQ3OCw3IEBAIERvY3VtZW50OjpEb2N1bWVudChG cmFtZSogZnJhbWUsIGNvbnN0IEtVUkwmIHVybCwgYm9vbCBpc1hIVE1MLCBib29sIGlzSFRNTCkK ICAgICAsIG1faXNWaWV3U291cmNlKGZhbHNlKQogICAgICwgbV9zYXdFbGVtZW50c0luS25vd25O YW1lc3BhY2VzKGZhbHNlKQogICAgICwgbV9pc1NyY2RvY0RvY3VtZW50KGZhbHNlKQotICAgICwg bV9yZW5kZXJWaWV3KDApCisgICAgLCBtX3JlbmRlcmVyKDApCiAgICAgLCBtX2V2ZW50UXVldWUo RG9jdW1lbnRFdmVudFF1ZXVlOjpjcmVhdGUodGhpcykpCiAgICAgLCBtX3dlYWtSZWZlcmVuY2Uo RG9jdW1lbnRXZWFrUmVmZXJlbmNlOjpjcmVhdGUodGhpcykpCiAgICAgLCBtX2lkQXR0cmlidXRl TmFtZShpZEF0dHIpCkBAIC0yMDU1LDEyICsyMDU1LDYgQEAgdm9pZCBEb2N1bWVudDo6Y2xlYXJT dHlsZVJlc29sdmVyKCkKICAgICBtX3N0eWxlUmVzb2x2ZXIuY2xlYXIoKTsKIH0KIAotdm9pZCBE b2N1bWVudDo6c2V0UmVuZGVyZXIoUmVuZGVyT2JqZWN0KiByZW5kZXJlcikKLXsKLSAgICBtX3Jl bmRlclZpZXcgPSB0b1JlbmRlclZpZXcocmVuZGVyZXIpOwotICAgIE5vZGU6OnNldFJlbmRlcmVy KHJlbmRlcmVyKTsKLX0KLQogdm9pZCBEb2N1bWVudDo6YXR0YWNoKCkKIHsKICAgICBBU1NFUlQo IWF0dGFjaGVkKCkpOwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvZG9tL0RvY3VtZW50Lmgg Yi9Tb3VyY2UvV2ViQ29yZS9kb20vRG9jdW1lbnQuaAppbmRleCAwMWE0NmJjNWFhNTY1MjNmMWQ3 OWM4OTJiZTgwNTRlOTU2NWQ3ZmI4Li4yZjg3MDdiZGQzMGIwMmJhY2JlMWNhZWJkYWJiYTYwMDIy MGQ1ZGIxIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9kb20vRG9jdW1lbnQuaAorKysgYi9T b3VyY2UvV2ViQ29yZS9kb20vRG9jdW1lbnQuaApAQCAtNTU2LDggKzU1NiwxNyBAQCBwdWJsaWM6 CiAKICAgICBSZW5kZXJBcmVuYSogcmVuZGVyQXJlbmEoKSB7IHJldHVybiBtX3JlbmRlckFyZW5h LmdldCgpOyB9CiAKLSAgICBSZW5kZXJWaWV3KiByZW5kZXJWaWV3KCkgY29uc3QgeyByZXR1cm4g bV9yZW5kZXJWaWV3OyB9Ci0gICAgdm9pZCBzZXRSZW5kZXJlcihSZW5kZXJPYmplY3QqKTsKKyAg ICAvLyBJbXBsZW1lbnRlZCBpbiBSZW5kZXJWaWV3LmggdG8gYXZvaWQgYSBjeWNsaWMgaGVhZGVy IGRlcGVuZGVuY3kgdGhpcyBqdXN0CisgICAgLy8gcmV0dXJucyByZW5kZXJlciBzbyBjYWxsZXJz IGNhbiBhdm9pZCB2ZXJib3NlIGNhc3RzLgorICAgIFJlbmRlclZpZXcqIHJlbmRlclZpZXcoKSBj b25zdDsKKworICAgIC8vIFNoYWRvdyB0aGUgaW1wbGVtZW50YXRpb25zIG9uIE5vZGUgdG8gcHJv dmlkZSBmYXN0ZXIgYWNjZXNzIGZvciBkb2N1bWVudHMuCisgICAgUmVuZGVyT2JqZWN0KiByZW5k ZXJlcigpIGNvbnN0IHsgcmV0dXJuIG1fcmVuZGVyZXI7IH0KKyAgICB2b2lkIHNldFJlbmRlcmVy KFJlbmRlck9iamVjdCogcmVuZGVyZXIpCisgICAgeworICAgICAgICBtX3JlbmRlcmVyID0gcmVu ZGVyZXI7CisgICAgICAgIE5vZGU6OnNldFJlbmRlcmVyKHJlbmRlcmVyKTsKKyAgICB9CiAKICAg ICB2b2lkIGNsZWFyQVhPYmplY3RDYWNoZSgpOwogICAgIEFYT2JqZWN0Q2FjaGUqIGF4T2JqZWN0 Q2FjaGUoKSBjb25zdDsKQEAgLTE0MzYsNyArMTQ0NSw3IEBAIHByaXZhdGU6CiAgICAgYm9vbCBt X3Nhd0VsZW1lbnRzSW5Lbm93bk5hbWVzcGFjZXM7CiAgICAgYm9vbCBtX2lzU3JjZG9jRG9jdW1l bnQ7CiAKLSAgICBSZW5kZXJWaWV3KiBtX3JlbmRlclZpZXc7CisgICAgUmVuZGVyT2JqZWN0KiBt X3JlbmRlcmVyOwogICAgIFJlZlB0cjxEb2N1bWVudEV2ZW50UXVldWU+IG1fZXZlbnRRdWV1ZTsK IAogICAgIFJlZlB0cjxEb2N1bWVudFdlYWtSZWZlcmVuY2U+IG1fd2Vha1JlZmVyZW5jZTsKZGlm ZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJWaWV3LmggYi9Tb3VyY2Uv V2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVmlldy5oCmluZGV4IDBmYzNhYjUyYjY5OTdlODA3ZDRk ZGExYjI1MzM2MjQ1YTE4Yzk5ODAuLjQ1NDQ0YWU1MGNjNTE1YmNkYzg3ZjgxMzkyZDJlNjI5YTBi NzExNDkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJWaWV3LmgK KysrIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclZpZXcuaApAQCAtMzQ5LDYgKzM0 OSwxMSBAQCBpbmxpbmUgY29uc3QgUmVuZGVyVmlldyogdG9SZW5kZXJWaWV3KGNvbnN0IFJlbmRl ck9iamVjdCogb2JqZWN0KQogLy8gVGhpcyB3aWxsIGNhdGNoIGFueW9uZSBkb2luZyBhbiB1bm5l Y2Vzc2FyeSBjYXN0Lgogdm9pZCB0b1JlbmRlclZpZXcoY29uc3QgUmVuZGVyVmlldyopOwogCitB TFdBWVNfSU5MSU5FIFJlbmRlclZpZXcqIERvY3VtZW50OjpyZW5kZXJWaWV3KCkgY29uc3QKK3sK KyAgICByZXR1cm4gdG9SZW5kZXJWaWV3KHJlbmRlcmVyKCkpOworfQorCiAvLyBTdGFjay1iYXNl ZCBjbGFzcyB0byBhc3Npc3Qgd2l0aCBMYXlvdXRTdGF0ZSBwdXNoL3BvcAogY2xhc3MgTGF5b3V0 U3RhdGVNYWludGFpbmVyIHsKICAgICBXVEZfTUFLRV9OT05DT1BZQUJMRShMYXlvdXRTdGF0ZU1h aW50YWluZXIpOwo=