currentThread is much slower than isMainThread. This leads to weird coding patterns (using isMainThread and that api can't even be used reliably in wtf because wtf may be used prior to initialize the main thread). In short, the slowness should be fixed.
Created attachment 100906 [details] Patch
Comment on attachment 100906 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100906&action=review > Source/JavaScriptCore/ChangeLog:9 > + With this change, currentThread is only 5% slower in debug and 10% in release mode. 5% slower and 10% slower than what?
(In reply to comment #2) > (From update of attachment 100906 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=100906&action=review > > > Source/JavaScriptCore/ChangeLog:9 > > + With this change, currentThread is only 5% slower in debug and 10% in release mode. > > 5% slower and 10% slower than what? Good point. I really butchered that line. I'll fix it before I check in. It should say "With this change, currentThread is 10% faster than isMainThread in release mode and only %5 slower than isMainThread in debug."
Strictly (paranoidly?) speaking, the code before the change allowed calling WTF::currentThread() before WTF::initializeThreading(). After change, the code assumes initializeThreading is always called first. Could you add an ASSERT to verify the assumption? It could be something like this: pthread_key_t ThreadIdentifierData::m_key = PTHREAD_KEYS_MAX; ... ThreadIdentifier ThreadIdentifierData::identifier() { ASSERT(m_key != PTHREAD_KEYS_MAX); ...
Landed (and fixed a header include issue): http://trac.webkit.org/changeset/91082 http://trac.webkit.org/changeset/91087 http://trac.webkit.org/changeset/91089 And rolled out due to gtk run time issues: http://trac.webkit.org/changeset/91094 Working on gtk fix before landing.
Landed again after the gtk fix. http://trac.webkit.org/changeset/91380
It appears to me that this changeset caused assertion failure in Chromium OS and Chromium Mac (possibility others). Here's a stack trace on Chromium OS: http://build.chromium.org/p/chromium/builders/Linux%20Tests%20%28ChromiumOS%20dbg%29%282%29/builds/2774/steps/browser_tests/logs/stdio ASSERTION FAILED: m_key != PTHREAD_KEYS_MAX third_party/WebKit/Source/JavaScriptCore/wtf/ThreadIdentifierDataPthreads.cpp(60) : static WTF::ThreadIdentifier WTF::ThreadIdentifierData::identifier() [7759:7768:0720/203836:986988360721:FATAL:utility_process_host.cc(39)] Check failed: !is_batch_mode_. Backtrace: base::debug::StackTrace::StackTrace() [0x9174f10] logging::LogMessage::~LogMessage() [0x9192f4d] UtilityProcessHost::~UtilityProcessHost() [0xb237a55] ChildProcessHost::OnChildDied() [0x9e77666] BrowserChildProcessHost::OnChildDied() [0xb104192] ChildProcessHost::ListenerHook::OnChannelError() [0x9e77a02] IPC::Channel::ChannelImpl::ClosePipeOnError() [0x9f7197e] IPC::Channel::ChannelImpl::OnFileCanReadWithoutBlocking() [0x9f71665] base::MessagePumpLibevent::FileDescriptorWatcher::OnFileCanReadWithoutBlocking() [0x9161ea8] base::MessagePumpLibevent::OnLibeventNotification() [0x9163cda] event_process_active [0x9829302] event_base_loop [0x9829647] base::MessagePumpLibevent::Run() [0x9163502] MessageLoop::RunInternal() [0x9197afd] MessageLoop::RunHandler() [0x91979d7] MessageLoop::Run() [0x919742b] base::Thread::Run() [0x91d77fb] base::Thread::ThreadMain() [0x91d79cb] base::(anonymous namespace)::ThreadFunc() [0x91d6d62] start_thread [0xf6c7196e] 0xf689bb5e [7759:7768:0720/203836:986988360721:FATAL:utility_process_host.cc(39)] Check failed: !is_batch_mode_. Backtrace: base::debug::StackTrace::StackTrace() [0x9174f10] logging::LogMessage::~LogMessage() [0x9192f4d] UtilityProcessHost::~UtilityProcessHost() [0xb237a55] ChildProcessHost::OnChildDied() [0x9e77666] BrowserChildProcessHost::OnChildDied() [0xb104192] ChildProcessHost::ListenerHook::OnChannelError() [0x9e77a02] IPC::Channel::ChannelImpl::ClosePipeOnError() [0x9f7197e] IPC::Channel::ChannelImpl::OnFileCanReadWithoutBlocking() [0x9f71665] base::MessagePumpLibevent::FileDescriptorWatcher::OnFileCanReadWithoutBlocking() [0x9161ea8] base::MessagePumpLibevent::OnLibeventNotification() [0x9163cda] event_process_active [0x9829302] event_base_loop [0x9829647] base::MessagePumpLibevent::Run() [0x9163502] MessageLoop::RunInternal() [0x9197afd] MessageLoop::RunHandler() [0x91979d7] MessageLoop::Run() [0x919742b] base::Thread::Run() [0x91d77fb] base::Thread::ThreadMain() [0x91d79cb] base::(anonymous namespace)::ThreadFunc() [0x91d6d62] start_thread [0xf6c7196e] 0xf689bb5e
Chromium Mac failures: http://build.chromium.org/p/chromium/builders/Mac%2010.6%20Tests%20%28dbg%29%284%29/builds/2134 http://build.chromium.org/p/chromium/builders/Mac%2010.6%20Tests%20%28dbg%29%282%29/builds/11687 http://build.chromium.org/p/chromium/builders/Mac%2010.5%20Tests%20%28dbg%29%283%29/builds/10242 http://build.chromium.org/p/chromium/builders/Mac%2010.6%20Tests%20%28dbg%29%281%29/builds/13178 http://build.chromium.org/p/chromium/builders/Mac%2010.6%20Tests%20%28dbg%29%282%29/builds/11687 http://build.chromium.org/p/chromium/builders/Mac%2010.6%20Tests%20%28dbg%29%283%29/builds/11138
Chromium Linux: http://build.chromium.org/p/chromium/builders/Linux%20Tests%20%28dbg%29%281%29/builds/9741 http://build.chromium.org/p/chromium/builders/Linux%20Tests%20%28dbg%29%28shared%29/builds/1463 http://build.chromium.org/p/chromium/builders/Linux%20Tests%20%28Views%20dbg%29%281%29/builds/3229 http://build.chromium.org/p/chromium/builders/Linux%20Tests%20%28Views%20dbg%29%282%29/builds/3185 http://build.chromium.org/p/chromium/builders/Linux%20Tests%20%28Views%20dbg%29%283%29/builds/3347 Chromium OS: http://build.chromium.org/p/chromium/builders/Linux%20Tests%20%28ChromiumOS%20dbg%29%282%29/builds/2774 http://build.chromium.org/p/chromium/builders/Linux%20Tests%20%28ChromiumOS%20dbg%29%283%29/builds/2776
Rolled out again :(. Now due to bad behavior in Chromium's IndexedDB tests.
(In reply to comment #10) > Rolled out again :(. > > Now due to bad behavior in Chromium's IndexedDB tests. Could it be that there's a bug in Chromium's IndexedDB implementation?
(In reply to comment #11) > (In reply to comment #10) > > Rolled out again :(. > > > > Now due to bad behavior in Chromium's IndexedDB tests. > > Could it be that there's a bug in Chromium's IndexedDB implementation? It suspect the tests (and with the rollout, the behavior is ok but unfortunate), but it could be the IndexedDB implementation. Something results in WTF::currentThread being called before WTF::initializeThreading (which only needs to be called once in the whole process -- of course the db stuff is in a different process in chromium I think).
Committed as http://trac.webkit.org/changeset/92154 (Now that I fixed Chromium downstream.)