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 throttle operator #515

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cmd-johnson
Copy link

I'm used to RxJS and loved the concept, so I wanted to use it in C++ as well. Seeing #501, I thought this would be a good opportunity for me to get a little deeper into understanding the implementation of RxCpp.

This is mostly a copy&paste of the debounce operator implementation, edited to implement throttle semantics (ref. RxJS).

The throttle operator re-emits the first value emitted by the source, then swallows all following emissions until the specified timeout has occurred. After that, the next value emitted by the source is emitted again and the cycle continues.

The RxJS implementation also supports emitting the last value emitted by the source when the timeout completes. If this is a feature you would like to see, I can add support for that as well.

@cmd-johnson
Copy link
Author

Hmmm, not quite sure why the Travis build fails. It's only one and it fails to install g++-8, which all other runners have had no problems with.

Copy link
Member

@kirkshoop kirkshoop left a comment

Choose a reason for hiding this comment

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

This is greatly appreciated!

auto localState = state;

const auto tp = localState->worker.now().time_since_epoch();
std::cout << "on_next(" << v << ") at " << tp.count() / 1000000 << " throttled: " << (localState->throttled ? "true" : "false") << std::endl;
Copy link
Member

Choose a reason for hiding this comment

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

please remove std::cout usage

auto work = [v, localState](const rxsc::schedulable&) {
auto produce_time = localState->worker.now() + localState->period;

std::cout << "scheduling unthrottle for " << (produce_time.time_since_epoch().count() / 1000000) << std::endl;
Copy link
Member

Choose a reason for hiding this comment

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

please remove std::cout usage

localState->throttled = true;

state->dest.on_next(v);

Copy link
Member

Choose a reason for hiding this comment

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

the call to on_next is not necessarily on the same worker. the following needs to be scheduled on the worker.

            if (!localState->throttled) {
                localState->throttled = true;

                state->dest.on_next(v);
            }

It would be more efficient to check throttled first and then only schedule state->dest.on_next(v); when throttled == false. To do that throttled would need to be std::atomic<bool>.

@antekone
Copy link

Any news on this PR? Could throttle be included in RxCpp?

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