Bug 115631 - Removing m_maxDeadCapacity condition in fast path in MemoryCache::prune().
Summary: Removing m_maxDeadCapacity condition in fast path in MemoryCache::prune().
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-06 03:55 PDT by Marcin Bychawski
Modified: 2013-10-31 07:26 PDT (History)
9 users (show)

See Also:


Attachments
patch (1.37 KB, patch)
2013-05-06 04:07 PDT, Marcin Bychawski
bfulgham: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Patch (1.52 KB, patch)
2013-10-31 02:29 PDT, Wojciech Bielawski
no flags Details | Formatted Diff | Diff
Patch (1.57 KB, patch)
2013-10-31 06:39 PDT, Wojciech Bielawski
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marcin Bychawski 2013-05-06 03:55:34 PDT
If the m_maxDeadCapacity == 0, MemoryCache::prune() always tries to prune Resources.
But if m_deadSize is also 0, pruning is unnecessary.
I removed one condition in the "fast path".

Is it possible that in some case this condition is required for properly work?
Comment 1 Marcin Bychawski 2013-05-06 04:07:40 PDT
Created attachment 200652 [details]
patch
Comment 2 Alexey Proskuryakov 2013-05-06 10:03:41 PDT
This patch is not currently marked for review.

Did you intend to submit it for review? You can still do that via Details link to the right of the patch.
Comment 3 Brent Fulgham 2013-05-08 23:23:03 PDT
Comment on attachment 200652 [details]
patch

This looks fine to me.  Is there any case we now fail, such as 'm_maxDeadCapacity == 0' and 'm_deadSize < 0' that might now get through and cause a problem?
Comment 4 Marcin Bychawski 2013-05-10 04:11:54 PDT
(In reply to comment #3)
> (From update of attachment 200652 [details])
> This looks fine to me.  Is there any case we now fail, such as 'm_maxDeadCapacity == 0' and 'm_deadSize < 0' that might now get through and cause a problem?

I don't see any case we now can fail.
m_deadSize is unsigned, so it can't be < 0.
When m_maxDeadCapacity == 0, we need to prune resources only when m_deadSize > 0, so there condition 'm_deadSize <= m_maxDeadCapacity' is enough.
Comment 5 Brent Fulgham 2013-10-30 10:35:16 PDT
Comment on attachment 200652 [details]
patch

I forgot about this!  Looks good. r=me
Comment 6 WebKit Commit Bot 2013-10-30 10:47:21 PDT
Comment on attachment 200652 [details]
patch

Rejecting attachment 200652 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 200652, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!.

Full output: http://webkit-queues.appspot.com/results/17138231
Comment 7 Wojciech Bielawski 2013-10-31 02:29:01 PDT
Created attachment 215636 [details]
Patch
Comment 8 Wojciech Bielawski 2013-10-31 06:39:41 PDT
Created attachment 215649 [details]
Patch
Comment 9 WebKit Commit Bot 2013-10-31 07:06:01 PDT
Comment on attachment 215649 [details]
Patch

Clearing flags on attachment: 215649

Committed r158357: <http://trac.webkit.org/changeset/158357>