64577 2011-07-14 17:47:24 -0700 currentThread is too slow! 2011-08-01 17:11:31 -0700 1 1 1 Unclassified WebKit Web Template Framework 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 64616 64871 64924 31639 1 levin levin ap dimich dslomov rniwa oldest_to_newest 437433 0 levin 2011-07-14 17:47:24 -0700 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. 437438 1 100906 levin 2011-07-14 17:54:19 -0700 Created attachment 100906 Patch 437439 2 100906 darin 2011-07-14 17:56:22 -0700 Comment on attachment 100906 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? 437442 3 levin 2011-07-14 18:00:32 -0700 (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." 437457 4 dimich 2011-07-14 18:37:07 -0700 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); ... 439802 5 levin 2011-07-20 07:52:08 -0700 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. 439889 6 levin 2011-07-20 11:22:15 -0700 Landed again after the gtk fix. http://trac.webkit.org/changeset/91380 440232 7 rniwa 2011-07-20 21:11:20 -0700 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 440235 8 rniwa 2011-07-20 21:17:22 -0700 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 440262 9 rniwa 2011-07-20 22:04:58 -0700 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 440265 10 levin 2011-07-20 22:12:10 -0700 Rolled out again :(. Now due to bad behavior in Chromium's IndexedDB tests. 440266 11 rniwa 2011-07-20 22:13:21 -0700 (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? 440268 12 levin 2011-07-20 22:18:04 -0700 (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). 445007 13 levin 2011-08-01 17:11:31 -0700 Committed as http://trac.webkit.org/changeset/92154 (Now that I fixed Chromium downstream.) 100906 2011-07-14 17:54:19 -0700 2011-07-14 17:56:22 -0700 Patch bug-64577-20110714175418.patch text/plain 4794 levin U3VidmVyc2lvbiBSZXZpc2lvbjogOTEwMzcKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDkz OGVjZTliZDE3OWZhMjZkNDQxYmE2MTI1ODI4NjZjZGM2MmU0NDUuLmU4Y2UxOWEwN2I5MDVkNDUx ZTI3MGY3YmYxNjY1ZDc5YmFkMWE4NjEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwpAQCAtMSwz ICsxLDIzIEBACisyMDExLTA3LTE0ICBEYXZpZCBMZXZpbiAgPGxldmluQGNocm9taXVtLm9yZz4K KworICAgICAgICBjdXJyZW50VGhyZWFkIGlzIHRvbyBzbG93IQorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NjQ1NzcKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGUgcHJvYmxlbSBpcyB0aGF0IGN1cnJlbnRU aHJlYWQgcmVzdWx0cyBpbiBhIHB0aHJlYWRfb25jZSBjYWxsIHdoaWNoIGFsd2F5cyB0YWtlcyBh IGxvY2suCisgICAgICAgIFdpdGggdGhpcyBjaGFuZ2UsIGN1cnJlbnRUaHJlYWQgaXMgb25seSA1 JSBzbG93ZXIgaW4gZGVidWcgYW5kIDEwJSBpbiByZWxlYXNlIG1vZGUuCisKKyAgICAgICAgKiB3 dGYvVGhyZWFkSWRlbnRpZmllckRhdGFQdGhyZWFkcy5jcHA6CisgICAgICAgIChXVEY6OlRocmVh ZElkZW50aWZpZXJEYXRhOjppbml0aWFsaXplT25jZSk6IFJlbW92ZSB0aGUgcHRocmVhZCBvbmNl IHN0dWZmCisgICAgICAgIHdoaWNoIGlzIG5vIGxvbmdlciBuZWVkZWQgYmVjYXVzZSB0aGlzIGlz IGNhbGxlZCBmcm9tIGluaXRpYWxpemVUaHJlYWRpbmcoKS4KKyAgICAgICAgKFdURjo6VGhyZWFk SWRlbnRpZmllckRhdGE6OmlkZW50aWZpZXIpOiBSZW1vdmUgdGhlIGluaXRpYWxpemVLZXlPbmNl IGNhbGwgYmVjYXVzZQorICAgICAgICBpbnRpYWxpemF0aW9uIG9mIHRoZSBwdGhyZWFkIGtleSBz aG91bGQgYWxyZWFkeSBiZSBkb25lLgorICAgICAgICAoV1RGOjpUaHJlYWRJZGVudGlmaWVyRGF0 YTo6aW5pdGlhbGl6ZSk6IERpdHRvLgorICAgICAgICAqIHd0Zi9UaHJlYWRJZGVudGlmaWVyRGF0 YVB0aHJlYWRzLmg6CisgICAgICAgICogd3RmL1RocmVhZGluZ1B0aHJlYWRzLmNwcDoKKyAgICAg ICAgKFdURjo6aW5pdGlhbGl6ZVRocmVhZGluZyk6IEFjcXVpcmUgdGhlIHB0aHJlYWQga2V5IGhl cmUuCisKIDIwMTEtMDctMTQgIEZpbGlwIFBpemxvICA8ZnBpemxvQGFwcGxlLmNvbT4KIAogICAg ICAgICBERkcgSklUIGNyYXNoZXMgb24gaG9zdCBjb25zdHJ1Y3RvciBjYWxscyBpbiBkZWJ1ZyBt b2RlLgpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3d0Zi9UaHJlYWRJZGVudGlm aWVyRGF0YVB0aHJlYWRzLmNwcCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvVGhyZWFkSWRl bnRpZmllckRhdGFQdGhyZWFkcy5jcHAKaW5kZXggZGNiYzk2ZmZjNDRiMDMyNjBhNTc3ZjJkOTU1 OWVhY2ZkZmMzNDMyNC4uZjYxYTU2ODBiNmQ3NmYxNmMxYmVjYmNlMDMxZTU4MGFhYjFjYzFlYSAx MDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3d0Zi9UaHJlYWRJZGVudGlmaWVyRGF0 YVB0aHJlYWRzLmNwcAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL1RocmVhZElkZW50 aWZpZXJEYXRhUHRocmVhZHMuY3BwCkBAIC0zOSw3ICszOSw2IEBACiBuYW1lc3BhY2UgV1RGIHsK IAogcHRocmVhZF9rZXlfdCBUaHJlYWRJZGVudGlmaWVyRGF0YTo6bV9rZXk7Ci1zdGF0aWMgcHRo cmVhZF9vbmNlX3Qgb25jZUNvbnRyb2wgPSBQVEhSRUFEX09OQ0VfSU5JVDsKIAogdm9pZCBjbGVh clB0aHJlYWRIYW5kbGVGb3JJZGVudGlmaWVyKFRocmVhZElkZW50aWZpZXIpOwogCkBAIC00OCw5 ICs0NywxNCBAQCBUaHJlYWRJZGVudGlmaWVyRGF0YTo6flRocmVhZElkZW50aWZpZXJEYXRhKCkK ICAgICBjbGVhclB0aHJlYWRIYW5kbGVGb3JJZGVudGlmaWVyKG1faWRlbnRpZmllcik7CiB9CiAK K3ZvaWQgVGhyZWFkSWRlbnRpZmllckRhdGE6OmluaXRpYWxpemVPbmNlKCkKK3sKKyAgICBpZiAo cHRocmVhZF9rZXlfY3JlYXRlKCZtX2tleSwgZGVzdHJ1Y3QpKQorICAgICAgICBDUkFTSCgpOwor fQorCiBUaHJlYWRJZGVudGlmaWVyIFRocmVhZElkZW50aWZpZXJEYXRhOjppZGVudGlmaWVyKCkK IHsKLSAgICBpbml0aWFsaXplS2V5T25jZSgpOwogICAgIFRocmVhZElkZW50aWZpZXJEYXRhKiB0 aHJlYWRJZGVudGlmaWVyRGF0YSA9IHN0YXRpY19jYXN0PFRocmVhZElkZW50aWZpZXJEYXRhKj4o cHRocmVhZF9nZXRzcGVjaWZpYyhtX2tleSkpOwogCiAgICAgcmV0dXJuIHRocmVhZElkZW50aWZp ZXJEYXRhID8gdGhyZWFkSWRlbnRpZmllckRhdGEtPm1faWRlbnRpZmllciA6IDA7CkBAIC01OSw4 ICs2Myw2IEBAIFRocmVhZElkZW50aWZpZXIgVGhyZWFkSWRlbnRpZmllckRhdGE6OmlkZW50aWZp ZXIoKQogdm9pZCBUaHJlYWRJZGVudGlmaWVyRGF0YTo6aW5pdGlhbGl6ZShUaHJlYWRJZGVudGlm aWVyIGlkKQogewogICAgIEFTU0VSVCghaWRlbnRpZmllcigpKTsKLQotICAgIGluaXRpYWxpemVL ZXlPbmNlKCk7CiAgICAgcHRocmVhZF9zZXRzcGVjaWZpYyhtX2tleSwgbmV3IFRocmVhZElkZW50 aWZpZXJEYXRhKGlkKSk7CiB9CiAKQEAgLTc5LDE4ICs4MSw2IEBAIHZvaWQgVGhyZWFkSWRlbnRp ZmllckRhdGE6OmRlc3RydWN0KHZvaWQqIGRhdGEpCiAgICAgcHRocmVhZF9zZXRzcGVjaWZpYyht X2tleSwgdGhyZWFkSWRlbnRpZmllckRhdGEpOwogfQogCi12b2lkIFRocmVhZElkZW50aWZpZXJE YXRhOjppbml0aWFsaXplS2V5T25jZUhlbHBlcigpCi17Ci0gICAgaWYgKHB0aHJlYWRfa2V5X2Ny ZWF0ZSgmbV9rZXksIGRlc3RydWN0KSkKLSAgICAgICAgQ1JBU0goKTsKLX0KLQotdm9pZCBUaHJl YWRJZGVudGlmaWVyRGF0YTo6aW5pdGlhbGl6ZUtleU9uY2UoKQotewotICAgIGlmIChwdGhyZWFk X29uY2UoJm9uY2VDb250cm9sLCBpbml0aWFsaXplS2V5T25jZUhlbHBlcikpCi0gICAgICAgIENS QVNIKCk7Ci19Ci0KIH0gLy8gbmFtZXNwYWNlIFdURgogCiAjZW5kaWYgLy8gVVNFKFBUSFJFQURT KQpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3d0Zi9UaHJlYWRJZGVudGlmaWVy RGF0YVB0aHJlYWRzLmggYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL1RocmVhZElkZW50aWZp ZXJEYXRhUHRocmVhZHMuaAppbmRleCBlZDFiYTJjYzAyODVhZjJiMTk2ZTY2Yzc3YmNiOWNkM2I2 MmNiMThlLi44NDM0OWEwY2RkYWFlYzg2MGJjYmU0YmMwYTcyNWJkMjc1ZTcyZWJkIDEwMDY0NAot LS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL1RocmVhZElkZW50aWZpZXJEYXRhUHRocmVh ZHMuaAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL1RocmVhZElkZW50aWZpZXJEYXRh UHRocmVhZHMuaApAQCAtNDIsNiArNDIsMTAgQEAgY2xhc3MgVGhyZWFkSWRlbnRpZmllckRhdGEg ewogcHVibGljOgogICAgIH5UaHJlYWRJZGVudGlmaWVyRGF0YSgpOwogCisgICAgLy8gT25lIHRp bWUgaW5pdGlhbGl6YXRpb24gZm9yIHRoaXMgY2xhc3MgYXMgYSB3aG9sZS4KKyAgICAvLyBUaGlz IG1ldGhvZCBtdXN0IGJlIGNhbGxlZCBiZWZvcmUgaW5pdGlhbGl6ZSgpIGFuZCBpdCBpcyBub3Qg dGhyZWFkLXNhZmUuCisgICAgc3RhdGljIHZvaWQgaW5pdGlhbGl6ZU9uY2UoKTsKKwogICAgIC8v IENyZWF0ZXMgYW5kIHB1dHMgYW4gaW5zdGFuY2Ugb2YgVGhyZWFkSWRlbnRpZmllckRhdGEgaW50 byB0aHJlYWQtc3BlY2lmaWMgc3RvcmFnZS4KICAgICBzdGF0aWMgdm9pZCBpbml0aWFsaXplKFRo cmVhZElkZW50aWZpZXIgaWRlbnRpZmllcik7CiAKQEAgLTYyLDkgKzY2LDYgQEAgcHJpdmF0ZToK ICAgICAvLyBUaHJlYWRJZGVudGlmaWVyIGZyb20gdGhlIHRocmVhZE1hcCwgY29tcGxldGluZyB0 aGUgY2xlYW51cC4KICAgICBzdGF0aWMgdm9pZCBkZXN0cnVjdCh2b2lkKiBkYXRhKTsKIAotICAg IHN0YXRpYyB2b2lkIGluaXRpYWxpemVLZXlPbmNlSGVscGVyKCk7Ci0gICAgc3RhdGljIHZvaWQg aW5pdGlhbGl6ZUtleU9uY2UoKTsKLQogICAgIFRocmVhZElkZW50aWZpZXIgbV9pZGVudGlmaWVy OwogICAgIGJvb2wgbV9pc0Rlc3Ryb3llZE9uY2U7CiAgICAgc3RhdGljIHB0aHJlYWRfa2V5X3Qg bV9rZXk7CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL1RocmVhZGluZ1B0 aHJlYWRzLmNwcCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvVGhyZWFkaW5nUHRocmVhZHMu Y3BwCmluZGV4IDkxNmFhMzY1NmRiYWM3ZjM4M2Y2M2U3NDNmZmViNmUyODEwZGRlYzUuLmQ1YWQ0 NmM0YmI2ZmU3Y2FjY2RiMWUxMTAwNWIzODE1OTdmNzM1OTcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9K YXZhU2NyaXB0Q29yZS93dGYvVGhyZWFkaW5nUHRocmVhZHMuY3BwCisrKyBiL1NvdXJjZS9KYXZh U2NyaXB0Q29yZS93dGYvVGhyZWFkaW5nUHRocmVhZHMuY3BwCkBAIC04MSw2ICs4MSw3IEBAIHZv aWQgaW5pdGlhbGl6ZVRocmVhZGluZygpCiAgICAgYXRvbWljYWxseUluaXRpYWxpemVkU3RhdGlj TXV0ZXggPSBuZXcgTXV0ZXg7CiAgICAgdGhyZWFkTWFwTXV0ZXgoKTsKICAgICBpbml0aWFsaXpl UmFuZG9tTnVtYmVyR2VuZXJhdG9yKCk7CisgICAgVGhyZWFkSWRlbnRpZmllckRhdGE6OmluaXRp YWxpemVPbmNlKCk7CiB9CiAKIHZvaWQgbG9ja0F0b21pY2FsbHlJbml0aWFsaXplZFN0YXRpY011 dGV4KCkK