Batch messages between Web Workers so there's only one ever in flight

[bemasc]

NOT FOR COMMIT

This change is designed to reduce the number of messages sent in very high load situations.

It appears to improve throughput in uproxy-churn by about 10%.

[willscott]

What is the 'not for commot' reservation? I'll provide comments regardless,
probably tomorrow.
On May 13, 2015 5:46 PM, "Benjamin M. Schwartz" notifications@github.com
wrote:

NOT FOR COMMIT

This change is designed to reduce the number of messages sent in very high
load situations.

It appears to improve throughput in uproxy-churn by about 10%.

You can view, comment on, or merge this pull request online at:

#269
Commit Summary

• Batch messages between Web Workers so there's only one ever in flight
• Try a more extreme batching.
• Cover a possible corner case, and add YUIDoc
• Fix logic bug: forgot to reset pendingAcks!
• Fix variable name error
• Raise the limit to 10 messages.
• A queue limit of 2 seems to be slightly faster than 10.

File Changes

• M src/link/worker.js
  https://github.com/freedomjs/freedom/pull/269/files#diff-0 (91)

Patch Links:

• https://github.com/freedomjs/freedom/pull/269.patch
• https://github.com/freedomjs/freedom/pull/269.diff

—
Reply to this email directly or view it on GitHub
#269.

[bemasc]

I just didn't want anyone to click "merge", or feel time pressure to
review, because the gain isn't as big as I thought, and it depends on the
limit in maybeSend in a way that I don't really understand.
On May 13, 2015 6:55 PM, "Will" notifications@github.com wrote:

What is the 'not for commot' reservation? I'll provide comments regardless,
probably tomorrow.
On May 13, 2015 5:46 PM, "Benjamin M. Schwartz" notifications@github.com
wrote:

NOT FOR COMMIT

This change is designed to reduce the number of messages sent in very
high
load situations.

It appears to improve throughput in uproxy-churn by about 10%.

You can view, comment on, or merge this pull request online at:

#269
Commit Summary

• Batch messages between Web Workers so there's only one ever in flight
• Try a more extreme batching.
• Cover a possible corner case, and add YUIDoc
• Fix logic bug: forgot to reset pendingAcks!
• Fix variable name error
• Raise the limit to 10 messages.
• A queue limit of 2 seems to be slightly faster than 10.

File Changes

• M src/link/worker.js
  https://github.com/freedomjs/freedom/pull/269/files#diff-0 (91)

Patch Links:

• https://github.com/freedomjs/freedom/pull/269.patch
• https://github.com/freedomjs/freedom/pull/269.diff

—
Reply to this email directly or view it on GitHub
#269.

—
Reply to this email directly or view it on GitHub
#269 (comment).

[agallant]

Just cleaning house, is this pull request still relevant? Thanks!

[bemasc]

I still think there might be a chance to gain ~5-10% on uProxy throughput with a technique like this. Now that we have better benchmarking tools it might be time to revisit this.

[agallant]

Sounds good, let me know if you'd like me to take a look.

[agallant]

Another cleaning-house revisit - if this is still relevant I'm happy to take a look. Thanks!

[bemasc]

Yeah, we don't need to hold this open. I'll reopen if we have evidence that it's worthwhile.