61017 – [chromium] Turn on WTF_MULTIPLE_THREADS.

RESOLVED FIXED 61017

[chromium] Turn on WTF_MULTIPLE_THREADS.

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

Summary [chromium] Turn on WTF_MULTIPLE_THREADS.

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-	Details Formatted Diff Diff
Enables WTF_MULTIPLE_THREADS in chromium port (8.09 KB, patch) 2011-07-27 14:44 PDT, Dmitry Lomov	levin: review-	Details Formatted Diff Diff
CR feedback (8.09 KB, patch) 2011-07-27 16:56 PDT, Dmitry Lomov	levin: review+ levin: commit-queue-	Details Formatted Diff Diff
CR feedback + wtfThreadData (9.02 KB, patch) 2011-07-27 17:13 PDT, Dmitry Lomov	no flags	Details Formatted Diff Diff
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.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware Unspecified

OS Unspecified

Product WebKit

Component Web Template Framework

Assignee

Dmitry Lomov

Reported

2011-05-17 19:08 PDT

Modified

2011-07-27 22:16 PDT History

CC List

6 users Show

URL

Keywords

Depends on

Blocks

61016

Dependencies

tree graph