Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 4.3k
ARROW-12056: [C++] Create sequencing AsyncGenerator#9779
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -280,6 +280,155 @@ AsyncGenerator<V> MakeMappedGenerator(AsyncGenerator<T> source_generator, | ||
| return MappingGenerator<T, V>(std::move(source_generator), std::move(map)); | ||
| } | ||
| /// \see MakeSequencingGenerator | ||
| template <typename T, typename ComesAfter, typename IsNext> | ||
| class SequencingGenerator { | ||
| public: | ||
| SequencingGenerator(AsyncGenerator<T> source, ComesAfter compare, IsNext is_next, | ||
| T initial_value) | ||
| : state_(std::make_shared<State>(std::move(source), std::move(compare), | ||
| std::move(is_next), std::move(initial_value))) {} | ||
| Future<T> operator()() { | ||
| { | ||
| auto guard = state_->mutex.Lock(); | ||
| // We can send a result immediately if the top of the queue is either an | ||
| // error or the next item | ||
| if (!state_->queue.empty() && | ||
| (!state_->queue.top().ok() || | ||
| state_->is_next(state_->previous_value, *state_->queue.top()))) { | ||
| auto result = std::move(state_->queue.top()); | ||
| if (result.ok()) { | ||
| state_->previous_value = *result; | ||
| } | ||
| state_->queue.pop(); | ||
| return Future<T>::MakeFinished(result); | ||
| } | ||
| if (state_->finished) { | ||
| return AsyncGeneratorEnd<T>(); | ||
| } | ||
| // The next item is not in the queue so we will need to wait | ||
| auto new_waiting_fut = Future<T>::Make(); | ||
| state_->waiting_future = new_waiting_fut; | ||
| guard.Unlock(); | ||
| state_->source().AddCallback(Callback{state_}); | ||
| return new_waiting_fut; | ||
| } | ||
| } | ||
| private: | ||
| struct WrappedComesAfter { | ||
| bool operator()(const Result<T>& left, const Result<T>& right) { | ||
| if (!left.ok() || !right.ok()) { | ||
| // Should never happen | ||
| return false; | ||
pitrou marked this conversation as resolved.
Outdated
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| return compare(*left, *right); | ||
| } | ||
| ComesAfter compare; | ||
| }; | ||
| struct State { | ||
| State(AsyncGenerator<T> source, ComesAfter compare, IsNext is_next, T initial_value) | ||
| : source(std::move(source)), | ||
| is_next(std::move(is_next)), | ||
| previous_value(std::move(initial_value)), | ||
| waiting_future(), | ||
| queue(WrappedComesAfter{compare}), | ||
| finished(false), | ||
| mutex() {} | ||
| AsyncGenerator<T> source; | ||
| IsNext is_next; | ||
| T previous_value; | ||
| Future<T> waiting_future; | ||
| std::priority_queue<Result<T>, std::vector<Result<T>>, WrappedComesAfter> queue; | ||
| bool finished; | ||
| util::Mutex mutex; | ||
| }; | ||
| class Callback { | ||
| public: | ||
| explicit Callback(std::shared_ptr<State> state) : state_(std::move(state)) {} | ||
| void operator()(const Result<T> result) { | ||
| Future<T> to_deliver; | ||
| bool finished; | ||
| { | ||
| auto guard = state_->mutex.Lock(); | ||
| bool ready_to_deliver = false; | ||
| if (!result.ok()) { | ||
| // Clear any cached results | ||
| while (!state_->queue.empty()) { | ||
| state_->queue.pop(); | ||
pitrou marked this conversation as resolved.
Outdated
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| ready_to_deliver = true; | ||
| state_->finished = true; | ||
| } else if (IsIterationEnd<T>(result.ValueUnsafe())) { | ||
| ready_to_deliver = state_->queue.empty(); | ||
| state_->finished = true; | ||
| } else { | ||
| ready_to_deliver = state_->is_next(state_->previous_value, *result); | ||
| } | ||
| if (ready_to_deliver && state_->waiting_future.is_valid()) { | ||
| to_deliver = state_->waiting_future; | ||
| if (result.ok()) { | ||
| state_->previous_value = *result; | ||
| } | ||
| } else { | ||
| state_->queue.push(result); | ||
| } | ||
| // Capture state_->finished so we can access it outside the mutex | ||
| finished = state_->finished; | ||
| } | ||
| // Must deliver result outside of the mutex | ||
| if (to_deliver.is_valid()) { | ||
| to_deliver.MarkFinished(result); | ||
| } else { | ||
| // Otherwise, if we didn't get the next item (or a terminal item), we | ||
| // need to keep looking | ||
| if (!finished) { | ||
lidavidm marked this conversation as resolved.
Outdated
Uh oh!There was an error while loading. Please reload this page. | ||
| state_->source().AddCallback(Callback{state_}); | ||
| } | ||
| } | ||
| } | ||
| private: | ||
| const std::shared_ptr<State> state_; | ||
| }; | ||
| const std::shared_ptr<State> state_; | ||
| }; | ||
| /// \brief Buffers an AsyncGenerator to return values in sequence order ComesAfter | ||
| /// and IsNext determine the sequence order. | ||
| /// | ||
| /// ComesAfter should be a BinaryPredicate that only returns true if a comes after b | ||
| /// | ||
| /// IsNext should be a BinaryPredicate that returns true, given `a` and `b`, only if | ||
| /// `b` follows immediately after `a`. It should return true given `initial_value` and | ||
| /// `b` if `b` is the first item in the sequence. | ||
| /// | ||
| /// This operator will queue unboundedly while waiting for the next item. It is intended | ||
| /// for jittery sources that might scatter an ordered sequence. It is NOT intended to | ||
| /// sort. Using it to try and sort could result in excessive RAM usage. This generator | ||
| /// will queue up to N blocks where N is the max "out of order"ness of the source. | ||
| /// | ||
| /// For example, if the source is 1,6,2,5,4,3 it will queue 3 blocks because 3 is 3 | ||
| /// blocks beyond where it belongs. | ||
| /// | ||
| /// This generator is not async-reentrant but it consists only of a simple log(n) | ||
| /// insertion into a priority queue. | ||
| template <typename T, typename ComesAfter, typename IsNext> | ||
| AsyncGenerator<T> MakeSequencingGenerator(AsyncGenerator<T> source_generator, | ||
| ComesAfter compare, IsNext is_next, | ||
| T initial_value) { | ||
| return SequencingGenerator<T, ComesAfter, IsNext>( | ||
| std::move(source_generator), std::move(compare), std::move(is_next), | ||
| std::move(initial_value)); | ||
| } | ||
| /// \see MakeTransformedGenerator | ||
| template <typename T, typename V> | ||
| class TransformingGenerator { | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -18,6 +18,7 @@ | ||
| #include <chrono> | ||
| #include <condition_variable> | ||
| #include <mutex> | ||
| #include <random> | ||
| #include <thread> | ||
| #include <unordered_set> | ||
| @@ -793,6 +794,118 @@ TEST(TestAsyncUtil, ReadaheadFailed) { | ||
| ASSERT_TRUE(IsIterationEnd(definitely_last)); | ||
| } | ||
| class SequencerTestFixture : public GeneratorTestFixture { | ||
| protected: | ||
| void RandomShuffle(std::vector<TestInt>& values) { | ||
| std::default_random_engine gen(seed_++); | ||
| std::shuffle(values.begin(), values.end(), gen); | ||
| } | ||
| int seed_ = 42; | ||
| std::function<bool(const TestInt&, const TestInt&)> cmp_ = | ||
| [](const TestInt& left, const TestInt& right) { return left.value > right.value; }; | ||
| // Let's increment by 2's to make it interesting | ||
| std::function<bool(const TestInt&, const TestInt&)> is_next_ = | ||
| [](const TestInt& left, const TestInt& right) { | ||
| return left.value + 2 == right.value; | ||
| }; | ||
| }; | ||
| TEST_P(SequencerTestFixture, SequenceBasic) { | ||
| // Basic sequencing | ||
| auto original = MakeSource({6, 4, 2}); | ||
| auto sequenced = MakeSequencingGenerator(original, cmp_, is_next_, TestInt(0)); | ||
| AssertAsyncGeneratorMatch({2, 4, 6}, sequenced); | ||
| // From ordered input | ||
| original = MakeSource({2, 4, 6}); | ||
| sequenced = MakeSequencingGenerator(original, cmp_, is_next_, TestInt(0)); | ||
| AssertAsyncGeneratorMatch({2, 4, 6}, sequenced); | ||
| } | ||
| TEST_P(SequencerTestFixture, SequenceLambda) { | ||
| auto cmp = [](const TestInt& left, const TestInt& right) { | ||
| return left.value > right.value; | ||
| }; | ||
| auto is_next = [](const TestInt& left, const TestInt& right) { | ||
| return left.value + 2 == right.value; | ||
| }; | ||
| // Basic sequencing | ||
| auto original = MakeSource({6, 4, 2}); | ||
| auto sequenced = MakeSequencingGenerator(original, cmp, is_next, TestInt(0)); | ||
| AssertAsyncGeneratorMatch({2, 4, 6}, sequenced); | ||
| } | ||
| TEST_P(SequencerTestFixture, SequenceError) { | ||
| { | ||
| auto original = MakeSource({6, 4, 2}); | ||
| original = FailsAt(original, 1); | ||
| auto sequenced = MakeSequencingGenerator(original, cmp_, is_next_, TestInt(0)); | ||
| auto collected = CollectAsyncGenerator(sequenced); | ||
| ASSERT_FINISHES_AND_RAISES(Invalid, collected); | ||
| } | ||
| { | ||
| // Failure should clear old items out of the queue immediately | ||
| // shared_ptr versions of cmp_ and is_next_ | ||
| auto cmp = cmp_; | ||
| std::function<bool(const std::shared_ptr<TestInt>&, const std::shared_ptr<TestInt>&)> | ||
| ptr_cmp = | ||
| [cmp](const std::shared_ptr<TestInt>& left, | ||
| const std::shared_ptr<TestInt>& right) { return cmp(*left, *right); }; | ||
| auto is_next = is_next_; | ||
| std::function<bool(const std::shared_ptr<TestInt>&, const std::shared_ptr<TestInt>&)> | ||
| ptr_is_next = [is_next](const std::shared_ptr<TestInt>& left, | ||
| const std::shared_ptr<TestInt>& right) { | ||
| return is_next(*left, *right); | ||
| }; | ||
| PushGenerator<std::shared_ptr<TestInt>> source; | ||
| auto sequenced = MakeSequencingGenerator( | ||
| static_cast<AsyncGenerator<std::shared_ptr<TestInt>>>(source), ptr_cmp, | ||
| ptr_is_next, std::make_shared<TestInt>(0)); | ||
| auto should_be_cleared = std::make_shared<TestInt>(4); | ||
| std::weak_ptr<TestInt> ref = should_be_cleared; | ||
| auto producer = source.producer(); | ||
| auto next_fut = sequenced(); | ||
| producer.Push(std::move(should_be_cleared)); | ||
| producer.Push(Status::Invalid("XYZ")); | ||
| ASSERT_TRUE(ref.expired()); | ||
| ASSERT_FINISHES_AND_RAISES(Invalid, next_fut); | ||
| } | ||
| { | ||
| // Failure should interrupt pumping | ||
| PushGenerator<TestInt> source; | ||
| auto sequenced = MakeSequencingGenerator(static_cast<AsyncGenerator<TestInt>>(source), | ||
| cmp_, is_next_, TestInt(0)); | ||
| auto producer = source.producer(); | ||
| auto next_fut = sequenced(); | ||
| producer.Push(TestInt(4)); | ||
| producer.Push(Status::Invalid("XYZ")); | ||
| producer.Push(TestInt(2)); | ||
| ASSERT_FINISHES_AND_RAISES(Invalid, next_fut); | ||
| // The sequencer should not have pulled the 2 out of the source because it should | ||
| // have stopped pumping on error | ||
| ASSERT_FINISHES_OK_AND_EQ(TestInt(2), source()); | ||
| } | ||
| } | ||
| TEST_P(SequencerTestFixture, SequenceStress) { | ||
| constexpr int NITEMS = 100; | ||
| for (auto task_index = 0; task_index < GetNumItersForStress(); task_index++) { | ||
| auto input = RangeVector(NITEMS, 2); | ||
| RandomShuffle(input); | ||
| auto original = MakeSource(input); | ||
| auto sequenced = MakeSequencingGenerator(original, cmp_, is_next_, TestInt(-2)); | ||
| AssertAsyncGeneratorMatch(RangeVector(NITEMS, 2), sequenced); | ||
| } | ||
| } | ||
| INSTANTIATE_TEST_SUITE_P(SequencerTests, SequencerTestFixture, | ||
| ::testing::Values(false, true)); | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm probably being dense or distracted, but I don't see the parameter being used anywhere. Could you point it out to me? :-S MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do my best to obscure it :). SequencerTestFixture extends GeneratorTestFixture which has a | ||
| TEST(TestAsyncIteratorTransform, SkipSome) { | ||
| auto original = AsyncVectorIt<TestInt>({1, 2, 3}); | ||
| auto filter = MakeFilter([](TestInt& t) { return t.value != 2; }); | ||
Uh oh!
There was an error while loading. Please reload this page.