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

[bugzilla-daemon at webkit.org]

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





--- Comment #46 from Balazs Kelemen <kbalazs at webkit.org>  2013-01-10 14:51:07 PST ---
(In reply to comment #44)
> (From update of attachment 103346 [details])
> 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.

This condition can only be true if the parser is stopped or it is under destruction, so we don't want to process this task anyway.

> 
> > 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.

I'm not sure how could we avoid that. Do you mean we should make sure that the thread will never fall asleep, i.e. it always have something to do? This seems tricky, I don't have any idea how to achieve that.

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