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

Add NewThrottle #427

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add NewThrottle #427

wants to merge 2 commits into from

Conversation

Lee-Minjea
Copy link

@Lee-Minjea Lee-Minjea commented Feb 14, 2024

Description

This PR adds lodash style 'throttle'.
NewThrottle function create a throttled instance that grants given functions are invoked only once in each interval.
It returns 2 functions, First one is throttled function and Second one is purge function which invokes given functions immediately and reset interval timer.

Related Issue

#396

Thank you.

@samber
Copy link
Owner

samber commented Jun 28, 2024

I think it has been implemented already: lo.NewDebounce.

@Lee-Minjea
Copy link
Author

I think it has been implemented already: lo.NewDebounce.

Throttle and debounce work slightly differently.
Debounce delays execution if there are new calls within a given time period, eventually it is executed only once at the end.
Throttle ensures that even if a function is called continuously, it will only be executed once within a given duration.

Copy link
Owner

@samber samber left a comment

Choose a reason for hiding this comment

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

I agree with you. This helper seems useful.

retry.go Outdated

if th.needInvoke || forcePurge {
for _, f := range th.callbacks {
go f()
Copy link
Owner

Choose a reason for hiding this comment

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

by default, i would make it synchronous and let the developer trigger a goroutine

WDYT ?

Copy link
Owner

Choose a reason for hiding this comment

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

can you explain why you trigger the callbacks when calling purge() ?

IMO, to force a call, we would code something like this:

throttle()
throttle() // skipped
throttle() // skipped
reset()
throttle()

Copy link
Author

@Lee-Minjea Lee-Minjea Aug 26, 2024

Choose a reason for hiding this comment

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

by default, i would make it synchronous and let the developer trigger a goroutine

WDYT ?

Your way is more simple and configuable,
I'll fix it that way.

can you explain why you trigger the callbacks when calling purge() ?

IMO, to force a call, we would code something like this:

throttle()
throttle() // skipped
throttle() // skipped
reset()
throttle()

I wanted to offer a function to force triggering callbacks and reset current interval schedule.
The usage of it is exactly same as you mentioned.
I thought, name 'reset' might imply resetting interval schedule, but not including calling the functions

But, If 'reset' is more appropriate conventionally, it could be changed.
Just give me your opinion.

retry.go Outdated
}
}

func (th *throttle) purge(forcePurge bool) {
Copy link
Owner

Choose a reason for hiding this comment

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

sed /purge/reset/

Copy link
Author

Choose a reason for hiding this comment

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

I change the function to 'reset'.


// NewThrottle creates a throttled instance that invokes given functions only once in every interval.
// This returns 2 functions, First one is throttled function and Second one is purge function which invokes given functions immediately and reset interval timer.
func NewThrottle(interval time.Duration, f ...func()) (func(), func()) {
Copy link
Owner

Choose a reason for hiding this comment

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

I'm also think about a NewThrottle(100ms, 3, cb) that would trigger the first 3 calls, then ignore others during 100ms. Like we do for a basic rate limiter.

Maybe it is a different helper.

WDYT?

Copy link
Author

Choose a reason for hiding this comment

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

I'm also think about a NewThrottle(100ms, 3, cb) that would trigger the first 3 calls, then ignore others during 100ms. Like we do for a basic rate limiter.

Maybe it is a different helper.

WDYT?

That would be a interesting option, but I am also afraid that might makes the function's signature complicated.
So I think, it's better to be implemented in other helper or function with different signature.

Copy link
Owner

Choose a reason for hiding this comment

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

2 helpers NewThrottle and NewThrottleWithCount ?

Copy link
Author

Choose a reason for hiding this comment

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

NewThrottleWithCount looks nice,
I'll add it

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.

2 participants