Closed Bug 861287 Opened 7 years ago Closed 7 years ago

Integrate IndexedDB into the gecko profiler

Categories

(Core :: Storage: IndexedDB, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla23

People

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

References

Details

Attachments

(1 file, 2 obsolete files)

Attached 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)
Attached 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+
Attached 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: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.