Closed Bug 1356556 Opened 3 years ago Closed 3 years ago
HTMLDNSPrefetch shouldn't create weak references all the time
Need to find some faster solution, so that BindToTree isn't so slow for links.
Hmm, that patch can't be enough. Need to check more than IsInComposedDoc
Attachment #8858314 - Attachment is obsolete: true
https://treeherder.mozilla.org/#/jobs?repo=try&revision=43a10f6dd9878c3c217dbdb15d7149acc8daba64 This is perhaps a bit annoying to review, but the idea is that there is global list of elements (in sPrefetches) and each document, which may have elements there, traverses/unlinks the relevant elements. The flag in nsIDocument is to optimize out unneeded traversing.
Comment on attachment 8858404 [details] [diff] [review] Add the elements to CC hmm, no, I'm still not happy. This makes us not leak, but elements may keep other things alive too long.
Since this is on top of the other Link related patch, asking review from you bz. Feel free to forward to someone else, perhaps peterv. This is perhaps a bit controversial because of RemoveUnboundLinks handling, but in practice should behave rather well. SnowWhiteKiller after all kills objects in batches. And avoiding the extra malloc WeakRef needs for Elements makes quite some difference in performance. Hopefully try likes this. https://treeherder.mozilla.org/#/jobs?repo=try&revision=35ff835b26e573e22f2f9300271022de948181b6
Attachment #8858460 - Flags: review?(bzbarsky)
Comment on attachment 8858460 [details] [diff] [review] link_dns_opt.diff This really needs a commit message describing the setup...
Attachment #8858460 - Flags: review?(bzbarsky) → review-
-m "Bug 1356556, nsHTMLDNSPrefetch can keep raw pointers to Links since the destructor of the Link clears the raw pointer, r=bz" That is the idea, and to optimize out going through the array for each Link dtor call, RemoveUnboundLinks is called and that removes all that once. https://treeherder.mozilla.org/#/jobs?repo=try&revision=6406d124311a4b549ccc164c969e8efcb91358f9
(Note the existing flags for handling deferred prefetch don't capture what we need here.)
Comment on attachment 8858514 [details] [diff] [review] link_dns_opt_3.diff See comment 7.
Attachment #8858514 - Flags: review?(bzbarsky)
Comment on attachment 8858514 [details] [diff] [review] link_dns_opt_3.diff > That is the idea, and to optimize out going through the array for each Link dtor call, RemoveUnboundLinks is called and that removes all that once. I'd really appreciate the commit message, explaining this. Or maybe the code comment in nsHTMLDNSPrefetch::LinkDestroyed could be expanded to something about how all links that are not in a composed doc can be cleared out of the list, and we're just using the opportunity we have here to do so. The aLink->IsInDNSPrefetch() check should probably happen before the LinkDestroyed() call, so we don't bother with the function call if not needed. r=me with that.
Attachment #8858514 - Flags: review?(bzbarsky) → review+
Added comment and improved commit message
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/03cdd7137ef1 use of weak references in nsHTMLDNSPrefetch is malloc heavy, so instead use raw pointers and a flag to tell that Link's destructor needs to clear the raw pointer, r=bz
You need to log in before you can comment on or make changes to this bug.