[Webkit-unassigned] [Bug 63531] Turn PreloadScanner implementation to a thread based solution

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 10 00:55:59 PST 2013


https://bugs.webkit.org/show_bug.cgi?id=63531





--- Comment #44 from Filip Pizlo <fpizlo at apple.com>  2013-01-10 00:57:50 PST ---
(From update of attachment 103346)
View in context: https://bugs.webkit.org/attachment.cgi?id=103346&action=review

> Source/WebCore/html/parser/HTMLPreloadScannerThread.cpp:58
> +    while (OwnPtr<HTMLPreloadScannerThread::ScanTask> task = m_tasks.waitForMessage()) {
> +        Mutex& scanTasksMutex = task->preloadScanner->scanTasksMutex();
> +        if (!scanTasksMutex.tryLock())
> +            continue;
> +        scan(task.get());
> +        scanTasksMutex.unlock();
> +    }

This seems like a really bad idea.  This looks like it will remove a task from the queue, but then if it cannot acquire the scanTasksMutex, it will just drop the task on the floor.

> Source/WebCore/html/parser/HTMLPreloadScannerThread.h:36
> +    MessageQueue<ScanTask> m_tasks;

I took a peek into MessageQueue, and it appears not to be particularly optimal.  It pretty much requires everything happening in lockstep: to add a task, you lock a lock, add things, signal, and unlock.  When you do this and the worker thread was asleep, you pay the price of it being awoken.  That's expensive.  The locking itself is probably expensive, but that's not nearly as bad.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list