|Summary:||[Cache] Add embedded profile to prune decoded resources more early|
|Product:||WebKit||Reporter:||Holger Freyther <zecke>|
|Component:||WebCore Misc.||Assignee:||Nobody <webkit-unassigned>|
|Severity:||Normal||CC:||abarth, ap, beidson, eric, ggaren, psolanki|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.5|
Description Holger Freyther 2009-10-08 07:36:43 PDT
Add a high watermark and if m_liveSize is bigger is bigger than this watermark ignore the calculated elapsedTime. This will make WebCore::Cache throw away decoded images even from within the paint routine which is helpful on sites with many images. This change does not make sure that m_liveSize will always stay below the watermark and is really only limited to decoded images. So if 3GB of CSS is downloaded m_liveSize will still grow beyond the high watermark.
Comment 1 Holger Freyther 2009-10-08 07:39:29 PDT
Created attachment 40869 [details] Add proof of concept for the approach I'm mostly seeking for feedback on if this is a good way to limit the memory usage. In the worse case the same image will be decoded more than once during painting. I would solve this in the ImageDecoder to asynchronously decode images. The proposed patch adds a new EMBEDDED_PROFILE that will in general prefer memory usage over speed. The proposed code will not add any branch or assignment for the normal build.
Comment 2 Adam Barth 2009-10-18 22:53:22 PDT
Comment on attachment 40869 [details] Add proof of concept for the approach This patch does not have a ChangeLog. If you'd like feedback on your patch, I recommend asking on IRC or emailing the relevant folks directly. Having this patch in the review queue is just making it hard to review complete patches.
Comment 3 Holger Freyther 2009-10-19 19:47:50 PDT
Created attachment 41475 [details] Add proof of concept for the approach. Same change with a ChangeLog entry.
Comment 4 Adam Barth 2009-11-30 12:19:20 PST
Attachment 41475 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 Done processing WebCore/loader/Cache.h WebCore/loader/Cache.cpp:279: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero]  WebCore/loader/Cache.cpp:280: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators]  Done processing WebCore/loader/Cache.cpp Total errors found: 2
Comment 5 Adam Barth 2009-12-04 13:02:08 PST
Comment on attachment 41475 [details] Add proof of concept for the approach. Please fix the style issues. Also, please use a const bool instead of EMBEDDED_PROFILE_CHECK. This code + if (EMBEDDED_PROFILE_CHECK elapsedTime < cMinDelayBeforeLiveDecodedPrune) looks super wonky. The compiler will optimize const bools properly.
Comment 6 Geoffrey Garen 2009-12-04 13:43:49 PST
If you want to add a feature with an #ifdef, all data members and functions should be surrounded by that #ifdef. I don't think EMBEDDED_PROFILE is a good name for this #ifdef, since some embedded profiles do not want to use it. Also, couldn't you write this whole patch in one line but just setting cMinDelayBeforeLiveDecodedPrune to 0?
Comment 7 Holger Freyther 2009-12-04 21:52:19 PST
(In reply to comment #5) > (From update of attachment 41475 [details]) > Please fix the style issues. Also, please use a const bool instead of > EMBEDDED_PROFILE_CHECK. This code > > + if (EMBEDDED_PROFILE_CHECK elapsedTime < cMinDelayBeforeLiveDecodedPrune) > > looks super wonky. The compiler will optimize const bools properly. The downside is that we normally don't use "const" for local variables.
Comment 8 Holger Freyther 2009-12-04 21:55:34 PST
(In reply to comment #6) > If you want to add a feature with an #ifdef, all data members and functions > should be surrounded by that #ifdef. > > I don't think EMBEDDED_PROFILE is a good name for this #ifdef, since some > embedded profiles do not want to use it. > > Also, couldn't you write this whole patch in one line but just setting > cMinDelayBeforeLiveDecodedPrune to 0? Good hint, let me research that.