Bug 30805

Summary: Add MessageQueue::removeWithPredicate to remove certain tasks without pulling them from the queue.
Product: WebKit Reporter: Dmitry Titov <dimich>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 30612    
Attachments:
Description Flags
Proposed patch. levin: review+, dimich: commit-queue-

Description Dmitry Titov 2009-10-26 21:07:52 PDT
As part of removing thread-safe refcounting from tasks, it makes sense to add the method on MessageQueue that would remove specified set of tasksk from the queue. Otherwise, this operation is done by removing tasksk from the queue and then re-inserting them back, which is longer, results in more locking operations and doesn't help bug 30612.
Comment 1 Dmitry Titov 2009-10-28 12:06:06 PDT
Created attachment 42047 [details]
Proposed patch.
Comment 2 David Levin 2009-10-28 13:54:28 PDT
Comment on attachment 42047 [details]
Proposed patch.

Just a few nits. Please fix and submit.


> diff --git a/JavaScriptCore/wtf/MessageQueue.h b/JavaScriptCore/wtf/MessageQueue.h
> +        void removeWithPredicate(Predicate&);

Consider using "removeIf" instead of "removeWithPredicate" (It is similar in naming to the findIf method on Deque).


> +    inline void MessageQueue<DataType>::removeWithPredicate(Predicate& predicate)
> +    {
> +        MutexLocker lock(m_mutex);
> +        DequeConstIterator<DataType> found = m_queue.end();
> +        while ((found = m_queue.findIf(predicate)) != m_queue.end()) {
> +            m_queue.remove(found); ASSERT(0);

Please remove "ASSERT(0);"



> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> +        (WebCore::SameDatabasePredicate::SameDatabasePredicate): Added predicate that flags the tasks belonging to specified database.

Consider: to "a" specified database.

> +        (WebCore::DatabaseThread::unscheduleDatabaseTasks): changed to use new removeWithPredicate method.

Consider: use "the" new removeWithPredicate
Comment 3 Dmitry Titov 2009-10-28 16:23:19 PDT
Fixed all and landed: http://trac.webkit.org/changeset/50247