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

Address Protocol delay_between_messages performance bottleneck #204

Merged
merged 1 commit into from
Jan 17, 2016

Conversation

mvollrath
Copy link
Contributor

This prevents performance problems when multiple clients are subscribing to high frequency topics.

Fixes #203

This prevents performance problems when multiple clients are subscribing to high frequency topics.

Fixes RobotWebTools#203
@mvollrath mvollrath changed the title Default Protocol delay_between_messages = 0 Address Protocol delay_between_messages performance bottleneck Jan 17, 2016
@T045T
Copy link
Contributor

T045T commented Jan 17, 2016

LGTM 👍

Does anyone else know of a system that would be negatively affected by this change?

@dabertram
Copy link
Contributor

after reading my comments about this parameter.. i think it should be okay to set this to 0.
If I remember correctly, (slow) clients can request their specific throttling rate anyways, right?

On January 17, 2016 4:17:14 PM CET, Nils Berg notifications@github.com wrote:

LGTM 👍

Does anyone else know of a system that would be negatively affected by
this change?


Reply to this email directly or view it on GitHub:
#204 (comment)

Sent from Mobile Device.

@T045T
Copy link
Contributor

T045T commented Jan 17, 2016

For inter-message throttling, yes.
There doesn't seem to be an easy way to set the message_intervall field in a roslibjs message, so large, fragmented messages might still overload a slow client, but even then it's still possible to just change the parameter on a per-server basis.

T045T added a commit that referenced this pull request Jan 17, 2016
Address Protocol delay_between_messages performance bottleneck
@T045T T045T merged commit 2e1b104 into RobotWebTools:develop Jan 17, 2016
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

3 participants