1092311 - Fix IndexedDB profiler helpers

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v0.1

This broke in bug 994190. Patch is mostly done, but still have a little bit to work out.

Comment 1

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

This should fix everything. The logging will be built in by default, piped to NSPR (so, subject to the NSPR environment variables), and the profiler markers can be enabled via preference changes.

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

Oops, other was a slightly older version. This is the right one.

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #1)
> This should fix everything. The logging will be built in by default, piped
> to NSPR (so, subject to the NSPR environment variables), and the profiler
> markers can be enabled via preference changes.

It's super awesome that the intent is for all builds to be PR_LOG enabled!

It looks like prlog.h still has the conditional in order to turn anything on:
  #if defined(DEBUG) || defined(FORCE_PR_LOG)

Should there be a FORCE_PR_LOG at the top of the profiler header or as a define in the moz.build or something?

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to Andrew Sutherland [:asuth] from comment #3)
> Should there be a FORCE_PR_LOG at the top of the profiler header or as a
> define in the moz.build or something?

I believe that was handled in bug 806819.

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

I misse(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #4)
> (In reply to Andrew Sutherland [:asuth] from comment #3)
> > Should there be a FORCE_PR_LOG at the top of the profiler header or as a
> > define in the moz.build or something?
> 
> I believe that was handled in bug 806819.

I missed that, thanks much for linking me to the relevant bug!  This will save me a lot of hassle next time I suspect something going wrong in mozStorage too.

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8520342 [details] [diff] [review]
Patch, v1

Review of attachment 8520342 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/indexedDB/IDBObjectStore.h
@@ +48,5 @@
>    : public nsISupports
>    , public nsWrapperCache
>  {
> +  // For AddOrPut() and DeleteInternal().
> +  friend class IDBCursor; 

moar whitespace

::: dom/indexedDB/IndexedDatabaseManager.cpp
@@ +774,5 @@
> +    sLoggingMode = Logging_Disabled;
> +    return;
> +  }
> +
> +  bool useProfiler = 

whitespace at EOL

::: dom/indexedDB/PBackgroundIDBSharedTypes.ipdlh
@@ +259,5 @@
> +struct LoggingInfo
> +{
> +  nsID backgroundChildLoggingId;
> +  int64_t nextTransactionSerialNumber;
> +  int64_t nextVersionChangeTransactionSerialNumber;

Comment that this is negative.

::: dom/indexedDB/ProfilerHelpers.h
@@ +68,5 @@
> +    // NSID_LENGTH counts the null terminator, SetLength() does not.
> +    SetLength(NSID_LENGTH - 1);
> +
> +    aID.ToProvidedString(
> +      *reinterpret_cast<char(*)[NSID_LENGTH]>(BeginWriting()));

OMGWTFBBQ

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/58982e7b5c45

Comment 8

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/efc1e23d4122 for https://treeherder.mozilla.org/logviewer.html#?job_id=4537005&repo=mozilla-inbound

Comment 9

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Interdiff

This has r=khuey.

Comment 10

[Ben Turner (not reading bugmail, use the needinfo flag!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/306a2f060615

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/306a2f060615