We've had lots of reports about slow database performance that turned out to be inaccurate. Oftentimes these assumptions have prevented us from hunting for and seeing less obvious bottlenecks. I'd like to make it very easy for developers to see where indexedDB is taking time so that we can focus our performance efforts. The first patch here is the basic helper stuff to get these annotations into the tree.

This is the full version that I'll check in, with all the annotations.

Comment on attachment 736888 [details] [diff] [review] Full patch Review of attachment 736888 [details] [diff] [review]: ----------------------------------------------------------------- r+ with the changes we discussed on IRC ::: dom/indexedDB/IDBCursor.cpp @@ +1026,5 @@ > > nsresult > ContinueHelper::PackArgumentsForParentProcess(CursorRequestParams& aParams) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", Thread-safety assertion. @@ +1050,1 @@ > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); Move this above. ::: dom/indexedDB/IDBIndex.cpp @@ +1191,5 @@ > > nsresult > GetKeyHelper::PackArgumentsForParentProcess(IndexRequestParams& aParams) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", Thread-safety assertion. @@ +1216,1 @@ > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); Move this above. @@ +1321,5 @@ > > nsresult > GetHelper::PackArgumentsForParentProcess(IndexRequestParams& aParams) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", Thread-safety assertion. @@ +1345,1 @@ > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); Move this above. @@ +1520,5 @@ > > nsresult > GetAllKeysHelper::PackArgumentsForParentProcess(IndexRequestParams& aParams) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", Thread-safety assertion. @@ +1552,1 @@ > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); Move this above. @@ +1674,5 @@ > > nsresult > GetAllHelper::PackArgumentsForParentProcess(IndexRequestParams& aParams) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", Thread-safety assertion. @@ +1706,1 @@ > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); Move this above. @@ +2011,5 @@ > > nsresult > OpenKeyCursorHelper::PackArgumentsForParentProcess(IndexRequestParams& aParams) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", ... @@ +2043,1 @@ > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); ... @@ +2350,5 @@ > > nsresult > OpenCursorHelper::PackArgumentsForParentProcess(IndexRequestParams& aParams) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", ... @@ +2382,1 @@ > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); ... @@ +2538,5 @@ > > nsresult > CountHelper::PackArgumentsForParentProcess(IndexRequestParams& aParams) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", ... @@ +2568,1 @@ > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); ... ::: dom/indexedDB/IDBObjectStore.cpp @@ +3121,5 @@ > > nsresult > AddHelper::PackArgumentsForParentProcess(ObjectStoreRequestParams& aParams) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", Add a thread-safety assertion? @@ +3181,1 @@ > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); Move this above the profiler label? @@ +3283,5 @@ > > nsresult > GetHelper::PackArgumentsForParentProcess(ObjectStoreRequestParams& aParams) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", Thread-safety assertion? @@ +3300,5 @@ > > AsyncConnectionHelper::ChildProcessSendResult > GetHelper::SendResponseToChildProcess(nsresult aResultCode) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", And here. @@ +3418,5 @@ > > nsresult > DeleteHelper::PackArgumentsForParentProcess(ObjectStoreRequestParams& aParams) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", And here. @@ +3435,5 @@ > > AsyncConnectionHelper::ChildProcessSendResult > DeleteHelper::SendResponseToChildProcess(nsresult aResultCode) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", And here. @@ +3498,5 @@ > > nsresult > ClearHelper::PackArgumentsForParentProcess(ObjectStoreRequestParams& aParams) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", And here. @@ +3517,1 @@ > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); Move this above the label? @@ +3729,5 @@ > nsresult > OpenCursorHelper::PackArgumentsForParentProcess( > ObjectStoreRequestParams& aParams) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", Thread-safety assertion. @@ +3761,1 @@ > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); Move this above. @@ +4176,5 @@ > > nsresult > GetAllHelper::PackArgumentsForParentProcess(ObjectStoreRequestParams& aParams) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", Thread-safety assertion. @@ +4208,1 @@ > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); Move this above. @@ +4382,5 @@ > > nsresult > CountHelper::PackArgumentsForParentProcess(ObjectStoreRequestParams& aParams) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", Thread-safety assertion. @@ +4412,1 @@ > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); Move this above. ::: dom/indexedDB/IDBTransaction.cpp @@ +827,5 @@ > CommitHelper::Run() > { > if (NS_IsMainThread()) { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", > + "IDBTransaction::CommitHelper::Run"); Missing filename. @@ +895,5 @@ > > return NS_OK; > } > > + PROFILER_LABEL("IndexedDB", "IDBTransaction::CommitHelper::Run"); Missing filename. ::: dom/indexedDB/OpenDatabaseHelper.cpp @@ +1569,1 @@ > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); Move this above. @@ +2199,5 @@ > > nsresult > OpenDatabaseHelper::EnsureSuccessResult() > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", Thread-safety assertion @@ +2340,5 @@ > > void > OpenDatabaseHelper::DispatchSuccessEvent() > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", ... @@ +2359,5 @@ > > void > OpenDatabaseHelper::DispatchErrorEvent() > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", ... ::: dom/indexedDB/TransactionThreadPool.cpp @@ +156,5 @@ > > nsresult > TransactionThreadPool::Cleanup() > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", "TransactionThreadPool::Cleanup"); Missing filename. @@ +420,5 @@ > void > TransactionThreadPool::AbortTransactionsForDatabase(IDBDatabase* aDatabase) > { > + PROFILER_MAIN_THREAD_LABEL("IndexedDB", > + "TransactionThreadPool::" Missing filename. ::: dom/quota/QuotaManager.cpp @@ +1885,5 @@ > > NS_IMETHODIMP > OriginClearRunnable::Run() > { > + PROFILER_LABEL("Quota", "OriginClearRunnable::Run"); Why are the various parts of this runnable not separated? @@ +2129,5 @@ > > NS_IMETHODIMP > AsyncUsageRunnable::Run() > { > + PROFILER_LABEL("Quota", "AsyncUsageRunnable::Run"); Ditto.

Initially I wasn't going to do the thread assertion stuff since it's already included in the macros, but after looking a little more I realized that our thread/process assertions have gotten a little inconsistent so I went ahead and made them all uniform. I also removed the filenames like we discussed, and we talked about splitting the rare-ish runnables into MAIN_THREAD and non.

This can't land until bug 734691.

Backed out, gcc didn't like it. https://hg.mozilla.org/integration/mozilla-inbound/rev/7b13433d8d0c

And https://hg.mozilla.org/integration/mozilla-inbound/rev/f0f0cf98b3cd for bustage

https://hg.mozilla.org/mozilla-central/rev/8dbc29b317ec https://hg.mozilla.org/mozilla-central/rev/f0f0cf98b3cd