NEW 118955
Strive to keep using the original request KURL instead of replacing it with identical copies. 
https://bugs.webkit.org/show_bug.cgi?id=118955
Summary Strive to keep using the original request KURL instead of replacing it with i...
Andreas Kling
Reported 2013-07-21 12:17:13 PDT
Strive to keep using the original request KURL instead of replacing it with identical copies.
Attachments
Patch idea (3.60 KB, patch)
2013-07-21 12:18 PDT, Andreas Kling 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2013-07-21 12:17:30 PDT
<rdar://problem/14504835>
Andreas Kling
Comment 2 2013-07-21 12:18:59 PDT
Created attachment 207211 [details] Patch idea
Andreas Kling
Comment 3 2013-07-21 16:06:04 PDT
This feels kinda hackish. Would love to hear from someone with loader expertise.
Sam Weinig
Comment 4 2013-07-21 18:26:53 PDT
I agree, it feel hackish.
Andreas Kling
Comment 5 2013-07-22 07:59:21 PDT
This is a 963 kB saver on theverge.com as well. Basically any site that uses huge data: URIs will see great improvement from fixing this.
Brady Eidson
Comment 6 2013-07-22 09:21:20 PDT
Comment on attachment 207211 [details] Patch idea Each of these 3 call sites focus on setting a KURL to another KURL. To be less hacky and to make experimenting with this optimization more centralized, I wonder if KURL's operator== should do this itself?
Brady Eidson
Comment 7 2013-07-22 09:22:29 PDT
That would definitely give us maximum memory savings, but would possibly be a performance hit... at which point we could scale it back and only do the optimization conditionally.
Andreas Kling
Comment 8 2013-07-22 09:24:01 PDT
(In reply to comment #6) > (From update of attachment 207211 [details]) > Each of these 3 call sites focus on setting a KURL to another KURL. > > To be less hacky and to make experimenting with this optimization more centralized, I wonder if KURL's operator== should do this itself? Something like.. if KURL::operator== will return true, it should first set m_string of both KURLs to whichever string has more refs?
Brady Eidson
Comment 9 2013-07-22 09:28:38 PDT
(In reply to comment #8) > (In reply to comment #6) > > (From update of attachment 207211 [details] [details]) > > Each of these 3 call sites focus on setting a KURL to another KURL. > > > > To be less hacky and to make experimenting with this optimization more centralized, I wonder if KURL's operator== should do this itself? > > Something like.. if KURL::operator== will return true, it should first set m_string of both KURLs to whichever string has more refs? Sure. Looking at operator== right now, it's so simplistic... those days are over! :)
Brady Eidson
Comment 10 2013-07-22 09:29:25 PDT
LOL - I bet doing this will uncover some threading errors. Perfect time for this!
Brady Eidson
Comment 11 2013-07-22 09:30:11 PDT
CRAP dude. sorry. I'm not awake yet. I meant operator=, not operator==.
Andreas Kling
Comment 12 2013-07-22 09:31:53 PDT
(In reply to comment #11) > CRAP dude. sorry. I'm not awake yet. > > I meant operator=, not operator==. That makes a bit more sense yeah :)
Andreas Kling
Comment 13 2013-08-02 06:57:51 PDT
Comment on attachment 207211 [details] Patch idea Retracting this patch for now.
Note You need to log in before you can comment on or make changes to this bug.