Skip to content

Conversation

@phausler
Copy link
Member

@phausler phausler commented Sep 7, 2022

This proposal formalizes rate limiting algorithms for debouncing and throttling.

Read the full proposal here

@FranzBusch
Copy link
Member

FranzBusch commented Sep 8, 2022

@phausler Debounce ought to implementable be without requiring the iterator to be Sendable. Have we looked into this?

@ktoso
Copy link
Member

ktoso commented Sep 8, 2022

It definitely is very weird/suspicious how many iterators (so, equivalents of a "subscription" intended for a single consumer) are becoming Sendable in this library 🤔

@twittemb
Copy link
Contributor

twittemb commented Sep 8, 2022

Hi

there was an issue regarding denouncing #174 should not we take a look beforehand ?

@phausler
Copy link
Member Author

phausler commented Sep 8, 2022

@FranzBusch the Sendable requirement definitely complicates the code considerably (again I think falls into the "this package should do it so other folks shouldn't" category). I am working on a version that reduces the task creation overhead and the sendable requirement to iterators.... but it makes the merge PR look simple in comparison... Sendable should be something part of the review.

@twittemb Good call, I think half of that report is resolved now due to fixes in clock, I have not yet had a chance to verify other parts of it. The behavior should not be a full predication on reviewing this however.

@FranzBusch
Copy link
Member

@FranzBusch the Sendable requirement definitely complicates the code considerably (again I think falls into the "this package should do it so other folks shouldn't" category). I am working on a version that reduces the task creation overhead and the sendable requirement to iterators.... but it makes the merge PR look simple in comparison... Sendable should be something part of the review.

I am with you that it complicates things but we really should fix this since it has significant usability downsides. Like you said "let's get it right once in this package". Happy to help with this!

@phausler
Copy link
Member Author

@swift-ci please test

@phausler phausler merged commit 7586d20 into apple:main Jan 3, 2023
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.

4 participants