Code review comment for lp:~osomon/webbrowser-app/limit-thumbnail-cache-size

Revision history for this message
Olivier Tilloy (osomon) wrote :

> 45 + m_thumbnailUtilsThread->deleteLater();
> I'd call m_thumbnailUtilsThread->wait() and delete the thread directly instead

Done. Thanks for the suggestion.

> 234 + QMetaObject::invokeMethod(&WebThumbnailUtils::instance(),
> "cacheThumbnail",
> You could add the connection type Qt::QueuedConnection, to highlight (and make
> sure) that it's asynchronous.

Although not strictly necessary, done.

> 148 + qint64 goal = MAX_CACHE_SIZE_IN_BYTES * 9 / 10;
> So the max cache size is 90% of 5MB? Why?

That was a mistake on my part. This code is not triggered only if the total cache size exceeds the limit. The goal is 90% of the limit to have some buffer, so that we do not need to expire entries every time a new thumbnail is generated (this is inspired by the implementation of the QNetworkDiskCache class).

« Back to merge proposal