<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">2016-10-13 6:46 GMT+02:00 Anand Sivaram <span dir="ltr"><<a href="mailto:aspnair@gmail.com" target="_blank">aspnair@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div style="font-family:monospace,monospace;font-size:small">What about having a global flag for the lock type?<br>Set it with g_object_set during the library initialization.<br></div><div style="font-family:monospace,monospace;font-size:small">It should have 3 values - Off, Per Agent, Global.<br></div><div style="font-family:monospace,monospace;font-size:small">Modify agent_lock() and agent_unlock() accordingly.<br><br></div><div style="font-family:monospace,monospace;font-size:small">That way if the applications using libnice implemented their own locking<br></div><div style="font-family:monospace,monospace;font-size:small">mechanism, that could be used seamlessly.<span class="HOEnZb"><font color="#888888"><br><br></font></span></div></div></blockquote><div><br></div><div><br></div><div>That looks like a smart approach, especially as it would be flexible: the off part is particularly interesting, as I have locks all over the place myself! :-). I'm still concerned about whether or not this can result in issues with the glib2 stuff libnice uses internally, though, and that applications don't have access to? There may be private methods invoked that need that locking, something the application from outside may not be able to protect.</div><div><br></div><div>Lorenzo</div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div style="font-family:monospace,monospace;font-size:small"><span class="HOEnZb"><font color="#888888"></font></span></div><span class="HOEnZb"><font color="#888888"><div style="font-family:monospace,monospace;font-size:small">anand<br></div><div style="font-family:monospace,monospace;font-size:small"><br></div><div style="font-family:monospace,monospace;font-size:small"><br></div><div style="font-family:monospace,monospace;font-size:small"><br></div></font></span></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On 12 October 2016 at 21:35, Lorenzo Miniero <span dir="ltr"><<a href="mailto:lminiero@gmail.com" target="_blank">lminiero@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Thanks for the context! (and sorry for the delay in this response...)<div><br></div><div>I'm not familiar enough with glib2, unfortunately: considering the patch dates back to about 7 years ago, is the problem the global muxed fixed still an issue with current versions? We did a quick and dirty test by turning those global agent locks to per-agent locks, where possible (that is, in methods that actually provided a pointer to an agent instance) and we did notice improvements in the performance (about twice as many sessions before it started to break down), but of course this means little if it risks causing those race conditions and crashes again. Is this road worth exploring? I'm willing to help and contribute to such work, of course, although I admit I'm not an expert on the libnice internals (but it's never too late to learn! ;-) ).</div><div><br></div><div>Thanks,</div><div>Lorenzo</div><div><br></div></div><div class="m_7447573976023040723HOEnZb"><div class="m_7447573976023040723h5"><div class="gmail_extra"><br><div class="gmail_quote">2016-10-05 13:09 GMT+02:00 Miguel París Díaz <span dir="ltr"><<a href="mailto:mparisdiaz@gmail.com" target="_blank">mparisdiaz@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Sorry for the copy-paste error.<br><br><br><div class="gmail_extra"><br><div class="gmail_quote"><span>2016-10-05 13:06 GMT+02:00 Miguel París Díaz <span dir="ltr"><<a href="mailto:mparisdiaz@gmail.com" target="_blank">mparisdiaz@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div><div>Good point Lorenzo!!!<br><br></div>This change was added in this commit:<br><br>commit 2faa76a4840100bebd9532bd47a275<wbr>5874c5b3a0<br>Author: Youness Alaoui <<a href="mailto:youness.alaoui@collabora.co.uk" target="_blank">youness.alaoui@collabora.co.u<wbr>k</a>><br>Date: Tue Jun 16 20:40:<a href="tel:33%202009%20-0400" value="+913320090400" target="_blank">33 2009 -0400</a><br><br> Use a global mutex for all nice agents and use g_source_is_destroyed to avoid the race condition of a source being destroyed while we wait for the mutex<br><br></div>And then the RecMutex was changed by a Mutex in:<br></div></div></blockquote></span><div><br>commit 1deee69325284c726c3a8380a7d583<wbr>9a51e20c48<br>Author: Olivier Crête <<a href="mailto:olivier.crete@collabora.com" target="_blank">olivier.crete@collabora.com</a>><br>Date: Mon Feb 24 18:51:31 2014 -0500<br><br> agent: Replace recursive mutex with non-recursive<br> <br> This should prevent us from re-adding re-entrancy in the future<br><br> </div><div><div class="m_7447573976023040723m_-909601598888611618h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div></div><br><br></div><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="m_7447573976023040723m_-909601598888611618m_-3479522597344256542gmail-h5">2016-10-05 11:45 GMT+02:00 Lorenzo Miniero <span dir="ltr"><<a href="mailto:lminiero@gmail.com" target="_blank">lminiero@gmail.com</a>></span>:<br></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="m_7447573976023040723m_-909601598888611618m_-3479522597344256542gmail-h5"><div dir="ltr">Hi all,<div><br></div><div>we've been working on some stress tests of our Janus WebRTC server, which uses libnice for all the ICE related stuff. While it works great in general, we reach a wall much sooner than we expect, and performance start to degrade quite a bit. By doing some digging and diagnostic in the code we wrote and the one we use, we found out something we thought was weird.</div><div><br></div><div>Specifically, we noticed that the internal method that is used to send packets, nice_agent_send_messages_nonbl<wbr>ocking (which nice_agent_send itself uses internally) has a lot of code wrapped in an agent_lock()/agent_unlock(), which locks/unlocks a global mutex, and this includes the process of physiclly sending the packet. This means that the whole library is locked until that code is called, which seems overkill. I guess the same happens in other parts of the code as well. I'd understand locking the specific agent if the purpose is thread-safety, but why lock ALL agents at the same time for each packet you send? I tried searching the archives for some info on that, but couldn't find any discussion related to this choice.</div><div><br></div><div>Since I'm already handling thread-safety for most stuff in my own code, is there any way for preventing this from happening? I guess that one possible way could be to retrieve the file descriptor from the agent object and send the packet manually myself, but while this may work in simple (host/prflx/srflx) connectivity, this would definitely not be the case when, for instance, TURN is involved, as that's one of the things the library would take care of for me.</div><div><br></div><div>Looking forward to your thoughts, thanks!<span class="m_7447573976023040723m_-909601598888611618m_-3479522597344256542gmail-m_-1552787990672576449HOEnZb"><font color="#888888"><br>Lorenzo</font></span></div></div>
<br></div></div><span class="m_7447573976023040723m_-909601598888611618m_-3479522597344256542gmail-">______________________________<wbr>_________________<br>
nice mailing list<br>
<a href="mailto:nice@lists.freedesktop.org" target="_blank">nice@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/nice" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/nice</a><br>
<br></span></blockquote></div><span class="m_7447573976023040723m_-909601598888611618m_-3479522597344256542gmail-"><br><br clear="all"><br>-- <br><div class="m_7447573976023040723m_-909601598888611618m_-3479522597344256542gmail-m_-1552787990672576449gmail_signature"><div dir="ltr">Miguel París Díaz<br>------------------------------<wbr>------------------------------<wbr>------------<br>Computer/Software engineer.<br>Researcher and architect in <a href="http://www.kurento.org" target="_blank">http://www.kurento.org</a><br><a href="http://twitter.com/mparisdiaz" target="_blank">http://twitter.com/mparisdiaz</a><br>------------------------------<wbr>------------------------------<wbr>------------<br></div></div>
</span></div>
</blockquote></div></div></div><div><div class="m_7447573976023040723m_-909601598888611618h5"><br><br clear="all"><br>-- <br><div class="m_7447573976023040723m_-909601598888611618m_-3479522597344256542gmail_signature"><div dir="ltr">Miguel París Díaz<br>------------------------------<wbr>------------------------------<wbr>------------<br>Computer/Software engineer.<br>Researcher and architect in <a href="http://www.kurento.org" target="_blank">http://www.kurento.org</a><br><a href="http://twitter.com/mparisdiaz" target="_blank">http://twitter.com/mparisdiaz</a><br>------------------------------<wbr>------------------------------<wbr>------------<br></div></div>
</div></div></div></div>
</blockquote></div><br></div>
</div></div><br>______________________________<wbr>_________________<br>
nice mailing list<br>
<a href="mailto:nice@lists.freedesktop.org" target="_blank">nice@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/nice" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/nice</a><br>
<br></blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>