{"action":"created","issue":{"url":"https://api.github.com/repos/atom/atom/issues/8783","labels_url":"https://api.github.com/repos/atom/atom/issues/8783/labels{/name}","comments_url":"https://api.github.com/repos/atom/atom/issues/8783/comments","events_url":"https://api.github.com/repos/atom/atom/issues/8783/events","html_url":"https://github.com/atom/atom/pull/8783","id":106380443,"number":8783,"title":"Recycle DOM nodes","user":{"login":"as-cii","id":482957,"avatar_url":"https://avatars.githubusercontent.com/u/482957?v=3","gravatar_id":"","url":"https://api.github.com/users/as-cii","html_url":"https://github.com/as-cii","followers_url":"https://api.github.com/users/as-cii/followers","following_url":"https://api.github.com/users/as-cii/following{/other_user}","gists_url":"https://api.github.com/users/as-cii/gists{/gist_id}","starred_url":"https://api.github.com/users/as-cii/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/as-cii/subscriptions","organizations_url":"https://api.github.com/users/as-cii/orgs","repos_url":"https://api.github.com/users/as-cii/repos","events_url":"https://api.github.com/users/as-cii/events{/privacy}","received_events_url":"https://api.github.com/users/as-cii/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":7,"created_at":"2015-09-14T16:33:12Z","updated_at":"2015-09-15T09:02:20Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/atom/atom/pulls/8783","html_url":"https://github.com/atom/atom/pull/8783","diff_url":"https://github.com/atom/atom/pull/8783.diff","patch_url":"https://github.com/atom/atom/pull/8783.patch"},"body":"After meeting with the Chrome team, we have found out that our GC times were quite severe when many DOM nodes were appended and removed from the tree (e.g. a typical scenario where you can see this in practice is scrolling).\r\n\r\nThis is a chart that shows how **master** behaves when scrolling down for a while:\r\n\r\n![before](https://cloud.githubusercontent.com/assets/482957/9855226/4436abee-5b0d-11e5-9755-ac33d579bc9b.png)\r\n\r\nAs you can see, the browser is triggering minor garbage collections during the scroll; at some point (which, _I think_, coincides with some Chrome threshold), a full GC is triggered and it takes quite a long time:\r\n\r\n![screen shot 2015-09-14 at 18 26 32](https://cloud.githubusercontent.com/assets/482957/9855367/28cbc33e-5b0e-11e5-8fcd-196c07bc2232.png)\r\n\r\nWith this PR we are introducing a DOM elements pool, which efficiently allocates and deallocates nodes to minimize GC times. The result is the following:\r\n\r\n![after](https://cloud.githubusercontent.com/assets/482957/9855407/4d56d2ac-5b0e-11e5-9fae-53be15539bb1.png)\r\n\r\n![screen shot 2015-09-14 at 18 25 47](https://cloud.githubusercontent.com/assets/482957/9855411/50a55104-5b0e-11e5-8fd9-0ca1d53213a8.png)\r\n\r\n:chart_with_downwards_trend: :racehorse: \r\n\r\nMoreover, we were able to ditch all the code that dealt with strings that contained null bytes (e.g. #8660) because now those characters are retained in the actual text nodes.\r\n\r\nSome other pair of :eyes: are most welcome, especially to look out for performance regressions (it _feels_ like `innerHTML` was a bit faster, but I couldn't confirm this).\r\n\r\nThanks!\r\n\r\n/cc: @atom/feedback @nathansobo "},"comment":{"url":"https://api.github.com/repos/atom/atom/issues/comments/140329593","html_url":"https://github.com/atom/atom/pull/8783#issuecomment-140329593","issue_url":"https://api.github.com/repos/atom/atom/issues/8783","id":140329593,"user":{"login":"as-cii","id":482957,"avatar_url":"https://avatars.githubusercontent.com/u/482957?v=3","gravatar_id":"","url":"https://api.github.com/users/as-cii","html_url":"https://github.com/as-cii","followers_url":"https://api.github.com/users/as-cii/followers","following_url":"https://api.github.com/users/as-cii/following{/other_user}","gists_url":"https://api.github.com/users/as-cii/gists{/gist_id}","starred_url":"https://api.github.com/users/as-cii/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/as-cii/subscriptions","organizations_url":"https://api.github.com/users/as-cii/orgs","repos_url":"https://api.github.com/users/as-cii/repos","events_url":"https://api.github.com/users/as-cii/events{/privacy}","received_events_url":"https://api.github.com/users/as-cii/received_events","type":"User","site_admin":false},"created_at":"2015-09-15T09:02:20Z","updated_at":"2015-09-15T09:02:20Z","body":"Thank you for sharing this super useful insight! That's an edge case I hadn't think of and which we should definitely address.\r\n\r\nI think we have a couple of ways in which this could be hypothetically fixed:\r\n\r\n1. Clearing out `DOMElementPool` when `fontSize` or `lineHeight` change. This is the most straightforward in my opinion and exploits the assumption that those parameters won't change that frequently under typical usage. \r\n2. Setting an absolute upper bound, as @lee-dohm proposed. This needs to be configured, though, and there's the risk of choosing a value that's not quite right.\r\n3. Setting a relative upper bound. The idea here could be to work with ratios, configuring the pool to rebalance automatically once the ratio reaches a certain value (i.e. `ratio` here could be defined as `freedElements / builtElements` for a given series of moments `t0, t1, t2...`). Although this is probably the best solution possible, I'd prefer to keep the pool code simple and straight to the point unless some more complex interaction becomes mandatory.\r\n\r\nI went ahead and implemented 1) in daf4316. @abe33: your experience with games would be super helpful here, so please let me know if you think there could be anything wrong with this solution. Thanks again! :+1: "}}