[webkit-reviews] review granted: [Bug 82816] presentationAttributeCacheMaximumSize is set too low : [Attachment 135207] Updated change. Add a timer based clearing mechanism per Antti's suggestion.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Apr 3 12:59:22 PDT 2012
Antti Koivisto <koivisto at iki.fi> has granted Julien Chaffraix
<jchaffraix at webkit.org>'s request for review:
Bug 82816: presentationAttributeCacheMaximumSize is set too low
https://bugs.webkit.org/show_bug.cgi?id=82816
Attachment 135207: Updated change. Add a timer based clearing mechanism per
Antti's suggestion.
https://bugs.webkit.org/attachment.cgi?id=135207&action=review
------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=135207&action=review
r=me but please consider the comments
> Source/WebCore/dom/StyledElement.cpp:77
> +class PresentationAttributeCacheClearTimer : public TimerBase {
> +public:
I think it would be nicer to have-a-timer than be-a-timer.
> Source/WebCore/dom/StyledElement.cpp:79
> + static void didHitPresentationAttributeCache()
> + {
The usual style is to have a singleton instance, something like
static PresentationAttributeCacheCleaner& presentationAttributeCacheCleaner()
{
DEFINE_STATIC_LOCAL(PresentationAttributeCacheCleaner, cleaner, ());
return cleaner;
}
and use non-static methods on that.
> Source/WebCore/dom/StyledElement.cpp:83
> + static PresentationAttributeCacheClearTimer timer;
Should use DEFINE_STATIC_LOCAL() (unless refactored as above).
> Source/WebCore/dom/StyledElement.cpp:93
> + static const unsigned presentationAttributeCacheCleanTimeInSeconds = 60;
> + static const int minPresentationAttributeCacheSizeForCleaning = 100;
> + static const unsigned minPresentationAttributeCacheHitCountPerMinute =
(100 * presentationAttributeCacheCleanTimeInSeconds) / 60;
How did you come up with these values?
min -> minimum
More information about the webkit-reviews
mailing list