914766 - [Contacts API] better debug function for the contacts API

Status: RESOLVED WONTFIX

(Core Graveyard :: DOM: Contacts, defect)

Description

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

In this bug, I'd like to do 2 things:

* investigate if we can basically revert bug 784746, but using a more clever debug function, where we don't concatenate strings before calling the function

* make the debug output depends on a pref

I have a patch with a try at https://tbpl.mozilla.org/?tree=Try&rev=66e1fd0a2de7
I'll test if we see a slowdown because of this.

Comment 1

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

Attachment: patch v1

Here are the timings, so there is a regression (with 500 :

without patch, debug is false: 1148ms
without patch, debug is true: 1234ms
with patch, debug is false: 1229ms
with patch, debug is true: 1350ms

If that's not acceptable, I can either put back all "if (DEBUG)" statements or find the ones that are especially slow (for example the ones that get a length, or are in a loop)

Comment 2

[Gregor Wagner [:gwagner]]

Comment on attachment 802534 [details] [diff] [review]
patch v1

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

A ~7% regression for some syntactic sugar is not ok with me.
I come from research where we publish papers if we find a 0.5% speedup for a certain benchmark :)

Comment 3

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

got it :)

I'll try to find the biggest culprits and otherwise just keep the debug configuration using the pref :)

thanks !

Comment 4

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

I tried again to measure and actually the time without the patch is not consistently so low.

Here are the 8 runs I did :

without patch, debug is false: 1203, 1167, 1353, 1098, 1251, 1123, 1225, 1257 => average 1209
with patch, debug is false: 1231, 1149, 1222, 1326, 1131, 1135, 1267, 1151 => average 1201

I think that if we regress in debug mode this is not a big deal.

Comment 5

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

Will wait for the WebIDL landing before moving forward here, because this will conflict.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

DOM: Contacts isn't used anymore. 
Closing all remaining bugs.