70352 2011-10-18 12:36:17 -0700 [chromium] Preserve offscreen tiles instead of immediately recycling them 2011-10-19 10:23:34 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 enne enne aelias enne jamesr nduca vangelis webkit.review.bot oldest_to_newest 486233 0 enne 2011-10-18 12:36:17 -0700 [chromium] Preserve offscreen tiles instead of immediately recycling them 486272 1 111493 enne 2011-10-18 13:27:35 -0700 Created attachment 111493 Patch 486275 2 enne 2011-10-18 13:32:12 -0700 I went back and forth a lot about whether to move the texture recycling or just remove it, and ended up on the removal end after some discussions with gman and nduca. Thoughts? 486291 3 111493 nduca 2011-10-18 13:53:29 -0700 Comment on attachment 111493 Patch Yay for changes with more deletions than additions! This looks awesome. 486341 4 111493 jamesr 2011-10-18 15:06:30 -0700 Comment on attachment 111493 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=111493&action=review This looks great! > Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp:-90 > - m_unusedTiles.clear(); can you delete m_unusedTiles from the class definition in the .h as well? It seems unused. 486361 5 aelias 2011-10-18 15:40:31 -0700 LGTM, I was about to make similar changes. Although I think we will probably need recycle textures in the future, in the TextureManager; I'd be surprised if every driver reacted elegantly to the churn. 486385 6 enne 2011-10-18 16:01:32 -0700 (In reply to comment #5) > LGTM, I was about to make similar changes. Although I think we will probably need recycle textures in the future, in the TextureManager; I'd be surprised if every driver reacted elegantly to the churn. Absolutely agreed that the TextureManager is a better place to do this recycling. My preference is to keep the code simple for now and, when we have a good use case that demonstrates a performance problem, use that to tune the right pool size, rather than prematurely doing it now via guesswork. 486411 7 enne 2011-10-18 16:18:23 -0700 Committed r97809: <http://trac.webkit.org/changeset/97809> 486450 8 vangelis 2011-10-18 17:10:15 -0700 (In reply to comment #7) > Committed r97809: <http://trac.webkit.org/changeset/97809> I like the cleanup but I think what we may see with this change is that our texture usage keep growing until we hit the texture reclaim limit. At that point, every time we cross a tile boundary we'll be deleting a row of texture tiles and allocating a new one. The good news is that the tiles we'll be deleting will likely be from a part of the page that's pretty far away from our current scroll position. The bad news is that we'll be churning (deleting and recreating) textures a lot. Our texture manager is unfortunately not able to recycle the actual GL texture objects. Given that our tiles are all the same size it would be very convenient if it did. Maybe that's a natural next step? 486871 9 enne 2011-10-19 10:23:34 -0700 (In reply to comment #8) > (In reply to comment #7) > > Committed r97809: <http://trac.webkit.org/changeset/97809> > > I like the cleanup but I think what we may see with this change is that our texture usage keep growing until we hit the texture reclaim limit. At that point, every time we cross a tile boundary we'll be deleting a row of texture tiles and allocating a new one. Yeah, having texture usage grow was my intention. Rather than have the layer do this reclamation, this will let the texture manager handle it naturally using LRU. If the active page has nothing else going on, I don't see any problem with texture usage sitting up at the soft limit, because it means that scrolling the entire page will be really fast. > Our texture manager is unfortunately not able to recycle the actual GL texture objects. Given that our tiles are all the same size it would be very convenient if it did. Maybe that's a natural next step? I am not entirely convinced of how much of a problem this is. We are already destroying and recreating tons of textures as layers are destroyed and created or as we switch tab focus. I think there are other tasks that are a higher priority to take care of, like improving the responsiveness of long commits by interleaving input and drawing. 111493 2011-10-18 13:27:35 -0700 2011-10-18 15:06:30 -0700 Patch bug-70352-20111018132734.patch text/plain 5011 enne U3VidmVyc2lvbiBSZXZpc2lvbjogOTc3ODMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCA5ZDVhMTk5MmMwOWJkMjk1 YTQ4MDZmNjZhZDgyNWNkZGE2ZjM3MjM1Li5kZGJmZWMwNGExMDhmZDIwMGQ1YWM2MzU4ZjA2Y2Ix YzQzM2QxNDI0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMzIgQEAKKzIwMTEtMTAtMTggIEFkcmll bm5lIFdhbGtlciAgPGVubmVAZ29vZ2xlLmNvbT4KKworICAgICAgICBbY2hyb21pdW1dIFByZXNl cnZlIG9mZnNjcmVlbiB0aWxlcyBpbnN0ZWFkIG9mIGltbWVkaWF0ZWx5IHJlY3ljbGluZyB0aGVt CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03MDM1Mgor CisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRlc3RlZCBi eSBleGlzdGluZyBsYXlvdXQgdGVzdHMuCisKKyAgICAgICAgUHJpb3IgdG8gaGF2aW5nIGEgVGV4 dHVyZU1hbmFnZXIgY2xhc3MsIHRpbGVkIGxheWVycyByZWN5Y2xlZCB0aGVpcgorICAgICAgICB0 ZXh0dXJlcyBpbnRlcm5hbGx5IHRvIGF2b2lkIHJlYWxsb2NhdGlvbi4gVW5mb3J0dW5hdGVseSwg aXQgcmVjeWNsZWQKKyAgICAgICAgdGhlc2UgdGlsZXMgYXMgc29vbiBhcyB0aGV5IHdlbnQgb2Zm c2NyZWVuLCBldmVuIGlmIHRoZXkgd2VyZSBzdGlsbAorICAgICAgICB2YWxpZC4gSW5zdGVhZCwg a2VlcCB0aWxlcyBhcm91bmQgZm9yZXZlciwgbGV0dGluZyB0aGUgVGV4dHVyZU1hbmFnZXIKKyAg ICAgICAgZGVjaWRlICh2aWEgTFJVKSB3aGVuIHRvIHJlY2xhaW0gdGhlIHVuZGVybHlpbmcgdGV4 dHVyZXMuIFRoaXMgd2lsbAorICAgICAgICBpbXByb3ZlIHNjcm9sbGluZyBwZXJmb3JtYW5jZSBk dWUgdG8gbm90IG5lZWRpbmcgdG8gcmVwYWludCB0aWxlcy4KKworICAgICAgICBUaGlzIGNoYW5n ZSBkb2VzIGxlYWQgdG8gbW9yZSB0ZXh0dXJlIHJlYWxsb2NhdGlvbiBmb3IgYW55IHRleHR1cmVz CisgICAgICAgIHRoYXQgZ2V0IHJlY2xhaW1lZCwgYnV0IHRoZSBjb21tYW5kIGJ1ZmZlciBpbXBs ZW1lbnRhdGlvbiBhbHJlYWR5CisgICAgICAgIHBvb2xzIGFuZCByZXVzZXMgdGV4dHVyZSBpZHMs IHNvIGl0IHNob3VsZCBub3QgaW50cm9kdWNlIGFkZGl0aW9uYWwKKyAgICAgICAgZmx1c2ggc3lu Y3MuIElmIHRoZXJlJ3MgYW55IHBlcmZvcm1hbmNlIHBlbmFsdHksIGl0J2xsIGJlIGF0IHRoZQor ICAgICAgICBkcml2ZXIgbGV2ZWwsIGJ1dCBJJ2QgcHJlZmVyIHRvIG1lYXN1cmUgdGhhdCB0aGVy ZSdzIGEgcHJvYmxlbSBiZWZvcmUKKyAgICAgICAgcHJlbWF0dXJlbHkgb3B0aW1pemluZyBhbmQg YWRkaW5nIGNvbXBsZXhpdHkgdG8gdGhlIFRleHR1cmVNYW5hZ2VyLgorCisgICAgICAgICogcGxh dGZvcm0vZ3JhcGhpY3MvY2hyb21pdW0vVGlsZWRMYXllckNocm9taXVtLmNwcDoKKyAgICAgICAg KFdlYkNvcmU6OlRpbGVkTGF5ZXJDaHJvbWl1bTo6Y2xlYW51cFJlc291cmNlcyk6CisgICAgICAg IChXZWJDb3JlOjpUaWxlZExheWVyQ2hyb21pdW06OmNyZWF0ZVRpbGUpOgorICAgICAgICAoV2Vi Q29yZTo6VGlsZWRMYXllckNocm9taXVtOjpwcmVwYXJlVG9VcGRhdGUpOgorICAgICAgICAqIHBs YXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL1RpbGVkTGF5ZXJDaHJvbWl1bS5oOgorCiAyMDExLTEw LTEzICBPamFuIFZhZmFpICA8b2phbkBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgaW1wbGVtZW50 IGZsZXgtZmxvdzpjb2x1bW4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dy YXBoaWNzL2Nocm9taXVtL1RpbGVkTGF5ZXJDaHJvbWl1bS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9w bGF0Zm9ybS9ncmFwaGljcy9jaHJvbWl1bS9UaWxlZExheWVyQ2hyb21pdW0uY3BwCmluZGV4IGQ4 MDc0ZmZhZGJlNDhkM2M1MjJkYTk2NjgxM2Q5OWFhOGM1Zjk0YTAuLjMyNzAyMjg2ODU4ODg3OTk5 NzUwM2UxNGYwOWQwOTRhZmE0N2IxN2MgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRm b3JtL2dyYXBoaWNzL2Nocm9taXVtL1RpbGVkTGF5ZXJDaHJvbWl1bS5jcHAKKysrIGIvU291cmNl L1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2hyb21pdW0vVGlsZWRMYXllckNocm9taXVtLmNw cApAQCAtODcsNyArODcsNiBAQCB2b2lkIFRpbGVkTGF5ZXJDaHJvbWl1bTo6Y2xlYW51cFJlc291 cmNlcygpCiAgICAgTGF5ZXJDaHJvbWl1bTo6Y2xlYW51cFJlc291cmNlcygpOwogCiAgICAgbV90 aWxlci5jbGVhcigpOwotICAgIG1fdW51c2VkVGlsZXMuY2xlYXIoKTsKICAgICBtX3BhaW50UmVj dCA9IEludFJlY3QoKTsKICAgICBtX3JlcXVlc3RlZFVwZGF0ZVJlY3QgPSBJbnRSZWN0KCk7CiB9 CkBAIC0yNjIsMzkgKzI2MSwxMyBAQCBVcGRhdGFibGVUaWxlKiBUaWxlZExheWVyQ2hyb21pdW06 OnRpbGVBdChpbnQgaSwgaW50IGopIGNvbnN0CiAKIFVwZGF0YWJsZVRpbGUqIFRpbGVkTGF5ZXJD aHJvbWl1bTo6Y3JlYXRlVGlsZShpbnQgaSwgaW50IGopCiB7Ci0gICAgUmVmUHRyPFVwZGF0YWJs ZVRpbGU+IHRpbGU7Ci0gICAgaWYgKG1fdW51c2VkVGlsZXMuc2l6ZSgpID4gMCkgewotICAgICAg ICB0aWxlID0gbV91bnVzZWRUaWxlcy5sYXN0KCkucmVsZWFzZSgpOwotICAgICAgICBtX3VudXNl ZFRpbGVzLnJlbW92ZUxhc3QoKTsKLSAgICAgICAgQVNTRVJUKHRpbGUtPnJlZkNvdW50KCkgPT0g MSk7Ci0gICAgfSBlbHNlIHsKLSAgICAgICAgVGV4dHVyZU1hbmFnZXIqIG1hbmFnZXIgPSB0ZXh0 dXJlTWFuYWdlcigpOwotICAgICAgICB0aWxlID0gYWRvcHRSZWYobmV3IFVwZGF0YWJsZVRpbGUo TWFuYWdlZFRleHR1cmU6OmNyZWF0ZShtYW5hZ2VyKSkpOwotICAgIH0KKyAgICBSZWZQdHI8VXBk YXRhYmxlVGlsZT4gdGlsZSA9IGFkb3B0UmVmKG5ldyBVcGRhdGFibGVUaWxlKE1hbmFnZWRUZXh0 dXJlOjpjcmVhdGUodGV4dHVyZU1hbmFnZXIoKSkpKTsKICAgICBtX3RpbGVyLT5hZGRUaWxlKHRp bGUsIGksIGopOwogICAgIHRpbGUtPm1fZGlydHlMYXllclJlY3QgPSBtX3RpbGVyLT50aWxlTGF5 ZXJSZWN0KHRpbGUuZ2V0KCkpOwogCiAgICAgcmV0dXJuIHRpbGUuZ2V0KCk7CiB9CiAKLXZvaWQg VGlsZWRMYXllckNocm9taXVtOjppbnZhbGlkYXRlVGlsZXMoY29uc3QgSW50UmVjdCYgY29udGVu dFJlY3QpCi17Ci0gICAgaWYgKCFtX3RpbGVyLT5udW1UaWxlcygpKQotICAgICAgICByZXR1cm47 Ci0KLSAgICBWZWN0b3I8Q0NMYXllclRpbGluZ0RhdGE6OlRpbGVNYXBLZXk+IHJlbW92ZUtleXM7 Ci0gICAgZm9yIChDQ0xheWVyVGlsaW5nRGF0YTo6VGlsZU1hcDo6Y29uc3RfaXRlcmF0b3IgaXRl ciA9IG1fdGlsZXItPnRpbGVzKCkuYmVnaW4oKTsgaXRlciAhPSBtX3RpbGVyLT50aWxlcygpLmVu ZCgpOyArK2l0ZXIpIHsKLSAgICAgICAgQ0NMYXllclRpbGluZ0RhdGE6OlRpbGUqIHRpbGUgPSBp dGVyLT5zZWNvbmQuZ2V0KCk7Ci0gICAgICAgIEludFJlY3QgdGlsZVJlY3QgPSBtX3RpbGVyLT50 aWxlQ29udGVudFJlY3QodGlsZSk7Ci0gICAgICAgIGlmICh0aWxlUmVjdC5pbnRlcnNlY3RzKGNv bnRlbnRSZWN0KSkKLSAgICAgICAgICAgIGNvbnRpbnVlOwotICAgICAgICByZW1vdmVLZXlzLmFw cGVuZChpdGVyLT5maXJzdCk7Ci0gICAgfQotCi0gICAgZm9yIChzaXplX3QgaSA9IDA7IGkgPCBy ZW1vdmVLZXlzLnNpemUoKTsgKytpKQotICAgICAgICBtX3VudXNlZFRpbGVzLmFwcGVuZChzdGF0 aWNfcG9pbnRlcl9jYXN0PFVwZGF0YWJsZVRpbGU+KG1fdGlsZXItPnRha2VUaWxlKHJlbW92ZUtl eXNbaV0uZmlyc3QsIHJlbW92ZUtleXNbaV0uc2Vjb25kKSkpOwotfQotCiB2b2lkIFRpbGVkTGF5 ZXJDaHJvbWl1bTo6aW52YWxpZGF0ZVJlY3QoY29uc3QgSW50UmVjdCYgY29udGVudFJlY3QpCiB7 CiAgICAgaWYgKCFtX3RpbGVyIHx8IGNvbnRlbnRSZWN0LmlzRW1wdHkoKSB8fCBtX3NraXBzRHJh dykKQEAgLTM1NSw5ICszMjgsNiBAQCB2b2lkIFRpbGVkTGF5ZXJDaHJvbWl1bTo6cHJlcGFyZVRv VXBkYXRlKGNvbnN0IEludFJlY3QmIGNvbnRlbnRSZWN0KQogICAgICAgICByZXR1cm47CiAgICAg fQogCi0gICAgLy8gSW52YWxpZGF0ZSBvbGQgdGlsZXMgdGhhdCB3ZXJlIHByZXZpb3VzbHkgdXNl ZCBidXQgYXJlbid0IGluIHVzZSB0aGlzCi0gICAgLy8gZnJhbWUgc28gdGhhdCB0aGV5IGNhbiBn ZXQgcmV1c2VkIGZvciBuZXcgdGlsZXMuCi0gICAgaW52YWxpZGF0ZVRpbGVzKGNvbnRlbnRSZWN0 KTsKICAgICBtX3RpbGVyLT5ncm93TGF5ZXJUb0NvbnRhaW4oY29udGVudFJlY3QpOwogCiAgICAg aWYgKCFtX3RpbGVyLT5udW1UaWxlcygpKSB7CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9w bGF0Zm9ybS9ncmFwaGljcy9jaHJvbWl1bS9UaWxlZExheWVyQ2hyb21pdW0uaCBiL1NvdXJjZS9X ZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL1RpbGVkTGF5ZXJDaHJvbWl1bS5oCmlu ZGV4IDNlYzgyYmZmYjVmNDkwNjg2MzBjYjkyNTViYTBjZWIwZDcyYzAwNzEuLmMwYzgxMDExZTg5 OGI1ZjIzNDMyYWY3YWMxMDk2MGNmMGFmMDU4YjkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3Jl L3BsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL1RpbGVkTGF5ZXJDaHJvbWl1bS5oCisrKyBiL1Nv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL1RpbGVkTGF5ZXJDaHJvbWl1 bS5oCkBAIC04MCw3ICs4MCw2IEBAIHByaXZhdGU6CiAKICAgICBVcGRhdGFibGVUaWxlKiB0aWxl QXQoaW50LCBpbnQpIGNvbnN0OwogICAgIFVwZGF0YWJsZVRpbGUqIGNyZWF0ZVRpbGUoaW50LCBp bnQpOwotICAgIHZvaWQgaW52YWxpZGF0ZVRpbGVzKGNvbnN0IEludFJlY3QmIGNvbnRlbnRSZWN0 KTsKIAogICAgIFRleHR1cmVNYW5hZ2VyKiB0ZXh0dXJlTWFuYWdlcigpIGNvbnN0OwogCg==