Skip to content

Conversation

@disq
Copy link
Member

@disq disq commented Jul 4, 2023

Alternative to #1058

@disq disq requested review from candiduslynx and hermanschaaf July 4, 2023 12:22
@disq disq requested a review from yevgenypats as a code owner July 4, 2023 12:23
@hermanschaaf
Copy link
Member

I haven't looked into it yet, but what does time.Ticker do if you pass in a 0 duration? If that works, maybe we can simplify by using that?

@disq
Copy link
Member Author

disq commented Jul 4, 2023

I haven't looked into it yet, but what does time.Ticker do if you pass in a 0 duration? If that works, maybe we can simplify by using that?

Looks like it will fire immediately (after a certain unspecified) delay:

func when(d Duration) int64 {
	if d <= 0 {
		return runtimeNano()
	}

@disq
Copy link
Member Author

disq commented Jul 4, 2023

We could also use .Reset(time.Duration) to re-use the same timer over and over again, would require a follow-up PR though... cc @candiduslynx

resources, sizeBytes = resources[:0], 0
}
tick = timer(w.batchTimeout)
tickClose()
Copy link
Contributor

Choose a reason for hiding this comment

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

you shouldn't need to manually close on every iteration

Copy link
Member Author

Choose a reason for hiding this comment

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

incoming commit:

type ticker interface {
	Chan() <-chan time.Time
	Reset(time.Duration)
	Stop() bool
}

@candiduslynx
Copy link
Contributor

@disq WDYT about e63bc5e instead?

@disq disq closed this Jul 4, 2023
@disq disq deleted the refactor_ticker_noleaks branch July 4, 2023 13:08
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.

3 participants