Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Agent for better performance and first class Trickle support #2

Closed
kixelated opened this issue Mar 4, 2019 · 2 comments
Closed

Comments

@kixelated
Copy link
Contributor

kixelated commented Mar 4, 2019

performance: ice/Agent.getBestPair

This function should be virtually instant but it takes 1.34% of my program's cpu time.

The issue is the message passing required because of ice/Agent.run. It would be much faster if this class used a mutex instead of taskLoop.

@Sean-Der Sean-Der transferred this issue from pion/webrtc Mar 26, 2019
@Sean-Der Sean-Der changed the title performance: ice/Agent.getBestPair Refactor Agent for better performance and first class Trickle support Aug 2, 2019
@Sean-Der
Copy link
Member

cpu.pprof.tar.gz

Here is a profile confirming. Most of our time is spend in sendto but that is another battle :)

Sean-Der added a commit that referenced this issue Mar 1, 2020
Remove taskChan and make .run just take an Agent wide mutex and run the
function. These is now a blocking operation so all channels used to
communicate from it must be buffered.

After this we will slowly remove usage of .run and make things more
thread safe.

Relates to #80, #67, #2
Sean-Der added a commit that referenced this issue Mar 1, 2020
Remove taskChan and make .run just take an Agent wide mutex and run the
function. These is now a blocking operation so all channels used to
communicate from it must be buffered.

After this we will slowly remove usage of .run and make things more
thread safe.

Relates to #80, #67, #2
Sean-Der added a commit that referenced this issue Mar 1, 2020
Remove taskChan and make .run just take an Agent wide mutex and run the
function. These is now a blocking operation so all channels used to
communicate from it must be buffered.

After this we will slowly remove usage of .run and make things more
thread safe.

Relates to #80, #67, #2
Sean-Der added a commit that referenced this issue Mar 2, 2020
Remove taskChan and make .run just take an Agent wide mutex and run the
function. These is now a blocking operation so all channels used to
communicate from it must be buffered.

After this we will slowly remove usage of .run and make things more
thread safe.

Relates to #80, #67, #2
Sean-Der added a commit that referenced this issue Mar 2, 2020
Remove taskChan and make .run just take an Agent wide mutex and run the
function. These is now a blocking operation so all channels used to
communicate from it must be buffered.

After this we will slowly remove usage of .run and make things more
thread safe.

Relates to #80, #67, #2
Sean-Der added a commit that referenced this issue Mar 2, 2020
Remove taskChan and make .run just take an Agent wide mutex and run the
function. These is now a blocking operation so all channels used to
communicate from it must be buffered.

After this we will slowly remove usage of .run and make things more
thread safe.

Relates to #80, #67, #2
Sean-Der added a commit that referenced this issue Mar 2, 2020
Remove taskChan and make .run just take an Agent wide mutex and run the
function. These is now a blocking operation so all channels used to
communicate from it must be buffered.

After this we will slowly remove usage of .run and make things more
thread safe.

Relates to #80, #67, #2
@Sean-Der
Copy link
Member

Sean-Der commented Mar 2, 2020

I have cleaned this up a bit, and now pion/ice isn't in the top 25. I am not going to work on this further.

I didn't end up removing .run, it is more then simple mutual exclusion it also allows cancellation of queued locks. I tried replacing things with a simple mutex, but ended up being more messy then I thought it would be.

For the cases where we want to return values from .run I have updated things to use atomics (instead of channels). .run itself is now blocking, and doesn't push the task across the channel. It instead calls the function directly.

@Sean-Der Sean-Der closed this as completed Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants