Summary: | [Cache] Add embedded profile to prune decoded resources more early | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Holger Freyther <zecke> | ||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | NEW --- | ||||||||
Severity: | Normal | CC: | abarth, ap, beidson, eric, ggaren, psolanki | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Holger Freyther
2009-10-08 07:36:43 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 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.
Created attachment 41475 [details]
Add proof of concept for the approach.
Same change with a ChangeLog entry.
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] [5]
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] [4]
Done processing WebCore/loader/Cache.cpp
Total errors found: 2
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.
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? (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. (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. |