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

`window_with_time_or_count` doesn't seem to cap by count #277

Closed
d-led opened this Issue Nov 25, 2016 · 6 comments

Comments

Projects
None yet
2 participants
@d-led

d-led commented Nov 25, 2016

Hi!

Am I wrong in assuming that in window with time or count the first window cap wins? I'd expect the windows to be capped at 100k, but they're clearly limited via the time. Or, maybe a scheduler is missing?

In the quick tryout of the window_with_time_or_count operator I'd expect the windows to be capped at 100k:

#include <rx.hpp>
#include <chrono>
#include <string>
#include <memory>
#include <fmt/ostream.h>

int main() {
    int counter = 0, count = 0;
    auto values = rxcpp::observable<>::range(1, 1000000)
        .window_with_time_or_count(std::chrono::seconds(1), 100000);
    values.
        subscribe(
            [&counter, &count](rxcpp::observable<int> window) {
        int id = counter++;
        printf("[window %d] Create window\n", id);
        window.count()
            .subscribe([](int c) {printf("Count in window: %d\n", c); });
        //...
        window.subscribe(
            [id, &count](int v) {count++;},
            [id, &count]() {printf("[window %d] OnCompleted: %d\n", id, count); });
    });
}

output:

[window 0] Create window
Count in window: 170306
Len: 910731 (123456789101112131415161718192021222324252...)
[window 0] OnCompleted: 170306
[window 1] Create window
Count in window: 172957
Len: 1037742 (170307170308170309170310170311170312170313...)
[window 1] OnCompleted: 343263
[window 2] Create window
Count in window: 171106
Len: 1026636 (343264343265343266343267343268343269343270...)
[window 2] OnCompleted: 514369
[window 3] Create window
Count in window: 173769
Len: 1042614 (514370514371514372514373514374514375514376...)
[window 3] OnCompleted: 688138
[window 4] Create window
Count in window: 175673
Len: 1054038 (688139688140688141688142688143688144688145...)
[window 4] OnCompleted: 863811
[window 5] Create window
Count in window: 136189
Len: 817135 (863812863813863814863815863816863817863818...)
[window 5] OnCompleted: 1000000
@kirkshoop

This comment has been minimized.

Show comment
Hide comment
@kirkshoop

kirkshoop Nov 25, 2016

Member

good catch! there is a bug in that operator.

the releasewindow() result, when the count is reached, is not being scheduled.

I will get a fix out soon.

Member

kirkshoop commented Nov 25, 2016

good catch! there is a bug in that operator.

the releasewindow() result, when the count is reached, is not being scheduled.

I will get a fix out soon.

kirkshoop pushed a commit to kirkshoop/RxCpp that referenced this issue Nov 25, 2016

@d-led

This comment has been minimized.

Show comment
Hide comment
@d-led

d-led Nov 25, 2016

superb! thanks! will wait for a nuget update :)

d-led commented Nov 25, 2016

superb! thanks! will wait for a nuget update :)

kirkshoop pushed a commit that referenced this issue Nov 25, 2016

Kirk Shoop
@kirkshoop

This comment has been minimized.

Show comment
Hide comment
@kirkshoop

kirkshoop Nov 26, 2016

Member

I pushed through a github release v3.0.0 and a nuget release v3.0.1 (I made a mistake with v3.0.0)

Try it out and see if it works!

Member

kirkshoop commented Nov 26, 2016

I pushed through a github release v3.0.0 and a nuget release v3.0.1 (I made a mistake with v3.0.0)

Try it out and see if it works!

@kirkshoop kirkshoop closed this Nov 26, 2016

@d-led

This comment has been minimized.

Show comment
Hide comment
@d-led

d-led Nov 26, 2016

Will do. Thanks!

d-led commented Nov 26, 2016

Will do. Thanks!

@d-led

This comment has been minimized.

Show comment
Hide comment
@d-led

d-led Nov 26, 2016

the window is now capped correctly, but getting a crash at the end if the window size is 0.

Ok, my bad: last() with an empty observable is a bad idea.

d-led commented Nov 26, 2016

the window is now capped correctly, but getting a crash at the end if the window size is 0.

Ok, my bad: last() with an empty observable is a bad idea.

@kirkshoop

This comment has been minimized.

Show comment
Hide comment
@kirkshoop

kirkshoop Nov 26, 2016

Member

Yes :)
I use start_with with the reduce operators that require at least one value.

Member

kirkshoop commented Nov 26, 2016

Yes :)
I use start_with with the reduce operators that require at least one value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment