Bug 104314 – Throw away StyleResolvers that haven't been used for a long time.

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Andreas Kling]

StyleResolver can get somewhat gigantic. Those that are not being used should be thrown away after some time since they can be reconstructed when needed.

[Andreas Kling]

Created attachment 178104 [details]
Proposed patch

[Antti Koivisto]

Comment on attachment 178104 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=178104&action=review

> Source/WebCore/dom/Document.cpp:4446
> +    // Throttle the calls to startOneShot() a bit since it's non-trivial on some platforms.
> +    if (now > m_lastStyleResolverAccessTime + 5) {
> +        m_styleResolverThrowawayTimer.startOneShot(60);
> +        m_lastStyleResolverAccessTime = now;
> +    }

Please use named constants.

[Andreas Kling]

Created attachment 178201 [details]
Land crab

[WebKit Review Bot]

Comment on attachment 178201 [details]
Land crab

Clearing flags on attachment: 178201

Committed r136956: <http://trac.webkit.org/changeset/136956>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Ojan Vafai]

Comment on attachment 178201 [details]
Land crab

View in context: https://bugs.webkit.org/attachment.cgi?id=178201&action=review

> Source/WebCore/dom/Document.cpp:4441
> +    static const int timeBeforeThrowingAwayStyleResolverAfterLastUseInSeconds = 60;
> +    static const int holdOffTimeBeforeReschedulingTimerInSeconds = 5;

Is it really important that these times be different? If not, you could make m_styleResolverThrowawayTimer a DeferrableOneShotTimer. Then this entire function would just be one line:
m_styleResolverThrowawayTimer.restart();

[Andreas Kling]

(In reply to comment #6)
> (From update of attachment 178201 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=178201&action=review
> 
> > Source/WebCore/dom/Document.cpp:4441
> > +    static const int timeBeforeThrowingAwayStyleResolverAfterLastUseInSeconds = 60;
> > +    static const int holdOffTimeBeforeReschedulingTimerInSeconds = 5;
> 
> Is it really important that these times be different? If not, you could make m_styleResolverThrowawayTimer a DeferrableOneShotTimer. Then this entire function would just be one line:
> m_styleResolverThrowawayTimer.restart();

I was not aware of DeferrableOneShotTImer. I'll post a patch to use that instead, thanks!