Skip to content
This repository has been archived by the owner on Feb 1, 2023. It is now read-only.

feat: debounce wants manually #255

Merged
merged 2 commits into from
Feb 13, 2020
Merged

feat: debounce wants manually #255

merged 2 commits into from
Feb 13, 2020

Conversation

Stebalien
Copy link
Member

Unfortunately, this is one of those cases in go where abstractions hurt more than they help.

This:

  • Makes it easy to send immediately if we wait too long and/or if we have enough to send.
  • Is significantly more efficient than the debounce library as it doesn't spin off a bunch of "after" timers.

fixes #245

This:

* Makes it easy to send immediately if we wait too long and/or if we have enough
to send.
* Is significantly more efficient than the debounce library as it doesn't spin
off a bunch of "after" timers.

fixes #245
Copy link
Contributor

@dirkmc dirkmc left a comment

Choose a reason for hiding this comment

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

Nice, this is a lot simpler 👍

I would like to abstract it out into its own class (in the messagequeue package) so as to more easily write tests for it. I will make a PR so we can see how it looks.

internal/messagequeue/messagequeue.go Show resolved Hide resolved
internal/messagequeue/messagequeue.go Outdated Show resolved Hide resolved
internal/messagequeue/messagequeue.go Outdated Show resolved Hide resolved
@dirkmc
Copy link
Contributor

dirkmc commented Feb 13, 2020

Note that this is slightly different from #254 in that sendMessageMaxDelay is only checked when the timer expires, so it's effectively a multiple of sendMessageDebounce. I prefer the simplicity of this approach so I think we should stick with it, but maybe we should explicitly define the max delay as a multiple of sendMessageDebounce, eg
sendMessageMaxDebounces := 100

^ nevermind I see that it preserves the time at which the debounce started so it's not necessarily a multiple of the debounce time

Copy link
Contributor

@dirkmc dirkmc left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Stebalien Stebalien merged commit f4c9702 into master Feb 13, 2020
@Stebalien
Copy link
Member Author

(we can continue discussing abstracting this in #256).

Jorropo pushed a commit to Jorropo/go-libipfs that referenced this pull request Jan 26, 2023
feat: debounce wants manually

This commit was moved from ipfs/go-bitswap@f4c9702
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Debounce wants
2 participants