libnice performance in networking I/O

[ikonst]

configure --disable-assert defines NDEBUG, which in turn is used in:

#ifdef NDEBUG
static inline gboolean nice_debug_is_enabled (void) { return FALSE; }
static inline void nice_debug (const char *fmt, ...) { }
#else
gboolean nice_debug_is_enabled (void);
void nice_debug (const char *fmt, ...) G_GNUC_PRINTF (1, 2);
#endif

nice_debug is being called repeatedly during network I/O in agent_recv_message_unlocked:

 nice_debug ("%s: Received %d valid messages of length %" G_GSIZE_FORMAT
      " from base socket %p.", G_STRFUNC, retval, message->length, nicesock);

Therefore, it will benefit performance to update the libnice recipe with --disable-assert. The calls to nice_debug take up 2% of out 13% CPU usage of iPhone 5s sending and receiving PCMA in loopback.

[superdump]

Then I think release mode builds could use this and debug could leave them there. I have been using nice debug printing recently for investigating ice problems and I would not want to disable it entirely. Maybe just a patch to remove that one debug print. That one isn't interesting in my opinion.

[ikonst]

Reported: https://bugs.freedesktop.org/show_bug.cgi?id=89278

I'll #ifdef 0 it myself through recipe patches for the time being.

[ikonst]

https://bugs.freedesktop.org/show_bug.cgi?id=89278 makes me sad.

Maybe we can just call nice_debug_disable first thing, then people can call nice_debug_enable when they actually need tracing — either from their code or via gdb/lldb? This is an effective way to not-vprintf.

[superdump]

I actually meant just throw in a patch locally into our build (cerbero recipe in @nirbheek's openwebrtc branch of his cerbero repo) so that we ignore just that debug output.

I think I prefer my suggestion purely for debug output purposes. It's never fun to have someone running an application, trying to debug it only to find out there's no debug logging present for you to enable. Your suggestion would disable all libnice debug logging and then we can get someone to check ICE debug stuff at runtime.

I don't like patching things locally in a project just to workaround such issues but I don't have any better ideas right now. Suggestions welcome.

[nirbheek]

I agree with @superdump here. Just patching out that line in a patch with a link to the FDO bug should suffice for now. It seems that Olivier wants to fix this bug properly by moving from nice_debug to g_debug, so this issue will be solved properly in a later release. We should just make sure to continue following up on the FDO bug. :)

[ikonst]

Will g_debug solve it by being very lightweight when the debugging output is disabled? (I'm not sure I understood that.)

What I suggested, this time around, is not to rebuild without debugging messages, but rather to call nice_debug_disable (a runtime function).

[nirbheek]

Will g_debug solve it by being very lightweight when the debugging output is disabled?

Mostly by being easy to enable/disable, with varying levels of debugging, and only having the overhead of one strdup() when disabled.

What I suggested, this time around, is not to rebuild without debugging messages, but rather to call nice_debug_disable (a runtime function).

The problem with that is that it disables all debugging messages without a way to enable it at runtime on a device, which seems like throwing the baby out with the bathtub. :) We could just ifdef out that one message for now.