Bug 39171

Summary: Reduce the size of CachedResource
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Page LoadingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, ossy, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch mjs: review+

Simon Fraser (smfr)
Reported 2010-05-15 18:47:25 PDT
The member variables of CachedResource could be rearranged for better padding.
Attachments
Patch (4.43 KB, patch)
2010-05-15 21:04 PDT, Simon Fraser (smfr) 		no flags
DetailsFormatted DiffDiff
Patch (18.64 KB, patch)
2010-05-15 22:57 PDT, Simon Fraser (smfr) 		mjs: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2010-05-15 21:04:25 PDT
Created attachment 56175 [details] Patch
Simon Fraser (smfr)
Comment 2 2010-05-15 21:36:43 PDT
Comment on attachment 56175 [details] Patch I'll do some bitfields to save more space.
Simon Fraser (smfr)
Comment 3 2010-05-15 22:57:35 PDT
Created attachment 56177 [details] Patch
Simon Fraser (smfr)
Comment 4 2010-05-15 22:58:15 PDT
This patch saves 32 bytes per CachedResource
Maciej Stachowiak
Comment 5 2010-05-15 23:01:54 PDT
Comment on attachment 56177 [details] Patch r=me
Simon Fraser (smfr)
Comment 6 2010-05-16 08:43:21 PDT
http://trac.webkit.org/changeset/59576
Simon Fraser (smfr)
Comment 7 2010-05-16 09:05:01 PDT
<rdar://problem/7990041>
WebKit Review Bot
Comment 8 2010-05-16 09:14:01 PDT
http://trac.webkit.org/changeset/59576 might have broken Qt Linux ARMv5 Release
Eric Seidel (no email)
Comment 9 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. :(
Eric Seidel (no email)
Comment 10 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.
Simon Fraser (smfr)
Comment 11 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?
Eric Seidel (no email)
Comment 12 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.
Note You need to log in before you can comment on or make changes to this bug.