Bug 61017

Summary: [chromium] Turn on WTF_MULTIPLE_THREADS.
Product: WebKit Reporter: David Levin <levin>
Component: Web Template FrameworkAssignee: Dmitry Lomov <dslomov>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, dglazkov, dslomov, levin, michaeln, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 61016    
Attachments:
Description Flags
Quick test on bots
webkit.review.bot: commit-queue-
Enables WTF_MULTIPLE_THREADS in chromium port
levin: review-
CR feedback
levin: review+, levin: commit-queue-
CR feedback + wtfThreadData none

David Levin
Reported 2011-05-17 19:08:49 PDT
We need to turn this on to make wtf safe to use in various threads. To do this, we likely need to move several items from initializeThreadingOnce to something in wtf (perhaps WTF::initializeThreading()). We also need to fix up FastMalloc.cpp (perhaps like https://bugs.webkit.org/attachment.cgi?id=92838&action=prettypatch but only for Chromium Windows not Windows in general.) There are some other helpful comments starting at https://bugs.webkit.org/show_bug.cgi?id=55728#c40.
Attachments
Quick test on bots (2.62 KB, patch)
2011-07-22 15:15 PDT, Dmitry Lomov 		webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Enables WTF_MULTIPLE_THREADS in chromium port (8.09 KB, patch)
2011-07-27 14:44 PDT, Dmitry Lomov 		levin: review-
DetailsFormatted DiffDiff
CR feedback (8.09 KB, patch)
2011-07-27 16:56 PDT, Dmitry Lomov 		levin: review+
levin: commit-queue-
DetailsFormatted DiffDiff
CR feedback + wtfThreadData (9.02 KB, patch)
2011-07-27 17:13 PDT, Dmitry Lomov 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Dmitry Lomov
Comment 1 2011-07-22 15:15:28 PDT
Created attachment 101776 [details] Quick test on bots
WebKit Review Bot
Comment 2 2011-07-22 16:17:03 PDT
Comment on attachment 101776 [details] Quick test on bots Attachment 101776 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9231232 New failing tests: animations/combo-transform-rotate+scale.html http/tests/inspector/console-xhr-logging.html http/tests/inspector/network/network-cachedresources-with-same-urls.html http/tests/inspector/change-iframe-src.html http/tests/inspector/network/network-disable-cache-memory.html http/tests/inspector/network/network-iframe-load-and-delete.html http/tests/inspector/network/network-disable-cache-xhrs.html http/tests/inspector/network/download.html http/tests/inspector/network-preflight-options.html animations/big-rotation.html http/tests/inspector/network/network-open-load-reopen.html animations/animation-add-events-in-handler.html http/tests/inspector/network/network-content-replacement-xhr.html http/tests/inspector/network/network-close-load-open.html animations/simultaneous-start-transform.html http/tests/inspector/resource-har-conversion.html http/tests/inspector/console-resource-errors.html http/tests/inspector/resource-parameters.html http/tests/inspector/network/network-clear-after-disabled.html animations/combo-transform-translate+scale.html
Dmitry Lomov
Comment 3 2011-07-27 14:44:01 PDT
Created attachment 102186 [details] Enables WTF_MULTIPLE_THREADS in chromium port No cq? - waiting for chromium trybots
David Levin
Comment 4 2011-07-27 15:02:05 PDT
Comment on attachment 102186 [details] Enables WTF_MULTIPLE_THREADS in chromium port View in context: https://bugs.webkit.org/attachment.cgi?id=102186&action=review > Source/JavaScriptCore/runtime/InitializeThreading.cpp:59 > initializeDates(); Why not remove this too? (and the String::Empty initialization). > Source/JavaScriptCore/wtf/FastMalloc.cpp:74 > +// * allocation of a reasonably complicated sfEtruct whoops > Source/JavaScriptCore/wtf/FastMalloc.cpp:83 > +#if OS(WINDOWS) and chromium? I think this already compiles fine on Windows for Apple's port. (Ditto for the OS(WINDOWS) below).
Dmitry Lomov
Comment 5 2011-07-27 15:16:40 PDT
Comment on attachment 102186 [details] Enables WTF_MULTIPLE_THREADS in chromium port View in context: https://bugs.webkit.org/attachment.cgi?id=102186&action=review >> Source/JavaScriptCore/runtime/InitializeThreading.cpp:59 >> initializeDates(); > > Why not remove this too? > > (and the String::Empty initialization). Done, sorry forgot to do that. >> Source/JavaScriptCore/wtf/FastMalloc.cpp:74 > > whoops Done >> Source/JavaScriptCore/wtf/FastMalloc.cpp:83 >> +#if OS(WINDOWS) > > and chromium? > > I think this already compiles fine on Windows for Apple's port. (Ditto for the OS(WINDOWS) below). Maybe !USE(PTHREADS)? I'll try to figure out the right set of defines
Dmitry Lomov
Comment 6 2011-07-27 16:56:59 PDT
Created attachment 102204 [details] CR feedback Fixed issues. I couldn't figure the better condition in FastMalloc.cpp than OS(WINDOWS) && PLATFORM(CHROMIUM). Chromium trybots are happy.
David Levin
Comment 7 2011-07-27 17:00:55 PDT
Comment on attachment 102204 [details] CR feedback View in context: https://bugs.webkit.org/attachment.cgi?id=102204&action=review > Source/JavaScriptCore/wtf/FastMalloc.cpp:87 > +#endif // OS(WINDOWS) You don't need to have these comments on the endif (and they are incorrect now anyway). > Source/JavaScriptCore/wtf/FastMalloc.cpp:113 > +static const LPVOID kTlsAllowValue = reinterpret_cast<LPVOID>(0); // must be zero Ideally make the comment look like a sentence. "Must be zero."
David Levin
Comment 8 2011-07-27 17:05:24 PDT
And move over wtfThreadData() as you mentioned.
Dmitry Lomov
Comment 9 2011-07-27 17:13:10 PDT
Created attachment 102205 [details] CR feedback + wtfThreadData
WebKit Review Bot
Comment 10 2011-07-27 22:16:37 PDT
Comment on attachment 102205 [details] CR feedback + wtfThreadData Clearing flags on attachment: 102205 Committed r91906: <http://trac.webkit.org/changeset/91906>
WebKit Review Bot
Comment 11 2011-07-27 22:16:42 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.