Integrate IndexedDB into the gecko profiler

RESOLVED FIXED in mozilla23

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bent.mozilla, Assigned: bent.mozilla)

Tracking

unspecified
mozilla23
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Posted patch Primary patch, v1 (obsolete) — Splinter Review
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.
Attachment #736887 - Flags: review?(khuey)
Posted patch Full patch (obsolete) — Splinter Review
This is the full version that I'll check in, with all the annotations.
Attachment #736888 - Flags: review?(khuey)
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.
Attachment #736888 - Flags: review?(khuey) → review+
Posted patch Final patchSplinter Review
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.
Attachment #736887 - Attachment is obsolete: true
Attachment #736888 - Attachment is obsolete: true
Attachment #737127 - Flags: review+
This can't land until bug 734691.
Depends on: 734691
https://hg.mozilla.org/mozilla-central/rev/8dbc29b317ec
https://hg.mozilla.org/mozilla-central/rev/f0f0cf98b3cd
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.