Bug 31693 - isMainThread() on Chromium (Mac and Linux) is so slow it timeouts LayoutTests..
Summary: isMainThread() on Chromium (Mac and Linux) is so slow it timeouts LayoutTests..
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-19 15:17 PST by Dmitry Titov
Modified: 2009-11-19 15:33 PST (History)
3 users (show)

See Also:


Attachments
Proposed patch. (1.90 KB, patch)
2009-11-19 15:21 PST, Dmitry Titov
levin: review+
dimich: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Titov 2009-11-19 15:17:12 PST
http://trac.webkit.org/changeset/51158 added ASSERT(isMainThread()) to TreeShared methods. This practically didn't slow down debug build on Mac and Windows, but on Chromium some layout tests started to timeout. For example, editing/selection/extend-selection.html execution time grew 15 times (!). This test operates with selection and it does a lot of operations like Position::previous() that churns refcout on Nodes.

Turned out the isMainThread() on pthread-based Chromium (Mac and Linux) involves mapping through threadMap, which in turn takes 2 Mutexes to lock - in identifierByPthreadHandle() and in iterator code.
By comparing handle itself rather then mapped Id the time required by isMainThread() goes back to reasonable.

Patch is coming.
Comment 1 Dmitry Titov 2009-11-19 15:21:03 PST
Created attachment 43530 [details]
Proposed patch.
Comment 2 Alexey Proskuryakov 2009-11-19 15:26:36 PST
Comment on attachment 43530 [details]
Proposed patch.

I wonder why you chose to rename the variable to mainThreadHandle. Mac man page for pthread_self says "returns the thread ID of the calling thread". Of course, "identifier" is taken already, but it's not a handle either.
Comment 3 Dmitry Titov 2009-11-19 15:33:56 PST
Landed: http://trac.webkit.org/changeset/51211