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

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

Projects

None yet

2 participants

@d-led
d-led commented Nov 25, 2016 edited

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
Contributor

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 kirkshoop added a commit to kirkshoop/RxCpp that referenced this issue Nov 25, 2016
@kirkshoop kirkshoop fix window_with_time_or_count
reported in #277
28e51ee
@d-led
d-led commented Nov 25, 2016 edited

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

@kirkshoop kirkshoop added a commit that referenced this issue Nov 25, 2016
@kirkshoop kirkshoop fix window_with_time_or_count
reported in #277
8290f92
@kirkshoop
Contributor

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
d-led commented Nov 26, 2016

Will do. Thanks!

@d-led
d-led commented Nov 26, 2016 edited

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
Contributor

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