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

peer: add data channel worker #136

Closed
wants to merge 1 commit into from
Closed

Conversation

daniel-abramov
Copy link
Contributor

@daniel-abramov daniel-abramov commented Feb 23, 2023

This relates to #133

The effect of this change is not as large as anticipated though. It turns out that for smaller messages in conferences with up to 6 people, the operation of sending to the data channel on the peer level took no more than 20µs on average. The peaks occur from time to time (up to 1 ms), but they are not as large, so the effect of this worker is not really that noticeable.

So now I question if it's worth it at all. Decided to file a PR to at least demonstrate how simple and small the change was and where it was done. We can close if we think that the complexity does not justify the improvement.

Please check the details: #133 (comment)

Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good: we can always try it & see if it makes a difference and back it out if it doesn't seem worthwhile.

@daniel-abramov
Copy link
Contributor Author

Closing this as obsolete as it turned out that it does not bring any significant performance, yet it complicates error propagation, so it seems like it's not worth it (the pros don't outweigh cons unfortunately).

@daniel-abramov daniel-abramov deleted the data-channel-worker branch March 21, 2023 13:03
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

Successfully merging this pull request may close these issues.

None yet

2 participants