Bug 39171 - Reduce the size of CachedResource
Summary: Reduce the size of CachedResource
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-05-15 18:47 PDT by Simon Fraser (smfr)
Modified: 2010-05-17 15:04 PDT (History)
4 users (show)

See Also:

Attachments
Patch (4.43 KB, patch)
2010-05-15 21:04 PDT, Simon Fraser (smfr) 		no flags Details | Formatted Diff | Diff
Patch (18.64 KB, patch)
2010-05-15 22:57 PDT, Simon Fraser (smfr) 		mjs: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2010-05-15 18:47:25 PDT 
The member variables of CachedResource could be rearranged for better padding.
Comment 1 Simon Fraser (smfr) 2010-05-15 21:04:25 PDT 
Created attachment 56175 [details]
Patch
Comment 2 Simon Fraser (smfr) 2010-05-15 21:36:43 PDT 
Comment on attachment 56175 [details]
Patch

I'll do some bitfields to save more space.
Comment 3 Simon Fraser (smfr) 2010-05-15 22:57:35 PDT 
Created attachment 56177 [details]
Patch
Comment 4 Simon Fraser (smfr) 2010-05-15 22:58:15 PDT 
This patch saves 32 bytes per CachedResource
Comment 5 Maciej Stachowiak 2010-05-15 23:01:54 PDT 
Comment on attachment 56177 [details]
Patch

r=me
Comment 6 Simon Fraser (smfr) 2010-05-16 08:43:21 PDT 
http://trac.webkit.org/changeset/59576
Comment 7 Simon Fraser (smfr) 2010-05-16 09:05:01 PDT 
<rdar://problem/7990041>
Comment 8 WebKit Review Bot 2010-05-16 09:14:01 PDT 
http://trac.webkit.org/changeset/59576 might have broken Qt Linux ARMv5 Release
Comment 9 Eric Seidel (no email) 2010-05-16 10:40:03 PDT 
Sorry, no.  Just the slave was lost.  I will fix sherriff bot to understand "slave lost" this week.  That or we'll remove the Qt builders since they cause sooooo much spam these days. :(
Comment 10 Eric Seidel (no email) 2010-05-16 10:45:09 PDT 
Comment on attachment 56177 [details]
Patch

Should we COMPILE_ASSERT that CachedResource is less than a certain size?  I'm not sure how many of these objects we typically keep around in process.
Comment 11 Simon Fraser (smfr) 2010-05-16 11:00:12 PDT 
We may have 1500+ alive at any one time with lots of windows/tabs open.

Yeah, I wish we had a way to identify memory regressions. Maybe we could have some tools that scrape the dSYM info for each build, and track size changes?
Comment 12 Eric Seidel (no email) 2010-05-17 15:04:46 PDT 
(In reply to comment #11)
> Yeah, I wish we had a way to identify memory regressions. Maybe we could have some tools that scrape the dSYM info for each build, and track size changes?

I think that that, or using COMPILE_ASSERTs on the various classes which we know we need to keep small are excellent ways to help prevent memory usage regressions.