|
Hi! I have started migrating from boost::fiber to tmc; after a few trivial helper classes I hit the first problem. I want to give std::ranges some async capabilities; first order is to consume a range in a tmc::task, pushing the values into a tmc::channel and consuming the channel as a range. Ideally, tmc::channel would implement begin()/end() (fiber::channel does that), I don't know if that's even possible. I tried having AI writing a simple range-wrapper around a chan_tok where the iterator would pull; I didn't even get that to compile because the pull would have to be co_awaited. My second try was creating a generator (Apple's clang doesn't have it yet, my implementation is based on https://www.scs.stanford.edu/~dm/blog/c++-coroutines.html) that loops over the channel and co_yields the values. That compiled, but asserted. the assert is in channel_storage::destroy, called from aw_pull_impl::await_resume. I don't understand coroutines internals enough; AI claims "TMC’s channel.pull() awaitable is only designed to be awaited from a TMC coroutine type (tmc::task / CoTask)". The documentation says a generator type is missing from tmc, is that what would be needed here? |
Replies: 16 comments
|
Re: implementing begin() / end() as async. Unforunately, only stackful coroutines (like boost::fiber) have the ability to suspend the caller implicitly. For C++20 stackless coroutines, you will need a Re: your range generator code. It looks good to me. You should be able to await That assert indicates that the element was never constructed, or was destructed twice. However, since your usage looks correct, I suspect a lifetime issue with the generator itself, since the awaitable exists inside the generator's coroutine frame. Are you able to share a small, complete example program that demonstrates the issue? One other thing to double check - the type A couple references: Notably, a Also note that the example syntax shown in that README does not work, as From the caller of the generator, this won't look too different than just using template <std::ranges::input_range Range>
requires std::movable<std::ranges::range_value_t<Range>>
[[nodiscard]] tmc::chan_tok<std::ranges::range_value_t<Range>> async(Range source)
{
using T = std::ranges::range_value_t<Range>;
auto channel = tmc::make_channel<T>();
tmc::post(ex_cpu(), [](Range source, auto channel) mutable -> CoTask<void> {
for (auto &&value : source)
co_await channel.push(std::move(value));
channel.close();
}(std::move(source), channel));
return channel;
}
tmc::task<void> caller() {
std::vector<Thing> things;
auto async_range = async(things);
auto data = co_await async_range.pull();
while (data.has_value()) {
process(data.value());
data = co_await async_range.pull();
}
} |
As to the range generator: the element wrongly destroyed belongs to the aw_pull_impl; I don't think it can be my generator code somehow not initializing the internal storage. But since you say it should work it tried debugging and anytime I hit breakpoints it mostly did work. in aw_pull_impl::await_ready, if there already is data ("elem->is_data_waiting()") it does work (I can force it by sleeping before the first co_await). When it actually suspends at the co_await, it asserts in the await_resume. As you said, claiming t was never constructed and indeed, breakpoints in channel_storage don't get hit and the value that has been moved from t is (in this case) an empty string. That's as far as I can get, to me it seems it shouldn't have resumed (yet)? This asserts almost always: |
|
Yes, I meant "stackful", thanks. I've fixed my initial comment, and also updated it with some more information and potential solutions. This code is a problem: auto vec = std::ranges::to<std::vector>(
TMCIssue::async(std::views::iota(0)
| std::views::take(4))
| std::views::transform([](int x) { return x + 1; })
);There's no What you need is an asynchronous generator. For this to work, you must have a To give a more detailed explanation of what's happening in your current implementation: You've implemented a synchronous generator type, but then you use auto data = co_await channel.pull(); // suspend point 1
while (data.has_value()) {
co_yield std::move(data.value()); // suspend point 2
data = co_await channel.pull(); // suspend point 3
}The only place where the caller should be allowed to resume the generator is at suspend point 2. When hitting suspend point 1 and 3, the caller needs to also suspend and wait for the other end of the channel (the What you have instead is the caller is attempting to resume the generator repeatedly, at all 3 suspend points. This results in the following sequence of calls (which I verified by adding some print statements):
However you usually don't get that far, as the assert occurs inside of step 4, when the channel sees that it was resumed without the data being present. This stops you immediately before the executor thread is allowed to try to resume the generator again. This is good, because you'd end up with multiple simultaneous copies of the generator running. |
|
What you're doing here doesn't make a lot of sense. You have a synchronous input (a range, which is a synchronous iterator / equivalent to a synchronous generator). Then you spawn an extra task to push the values into a channel, so you can make it awaitable. But then you want to consume it in a synchronous manner. I suggest for this use case that you just use the original range directly. If your input is async (the original input is from some async stream like a file) and you want to consume the outputs in a synchronous manner, then you'd need to block on pulling data out of the channel. Currently I don't provide any mechanism for this, so you would need to use If your input is async, and you want to consume the outputs asynchronously, then just await the data source directly and do the processing inline. Or if you need to fan out data to multiple consumers, then you could return the |
|
Thanks for all the explanations; it seems what I am trying to do is just not possible. my use case: I have a zip file containing a huge (300GB) csv file. I have a pipeline like this: unzip_4MBData_chunks_generator | convert_into_records | chunk_by_key | compress | store_in_db This all works with my generator and std::ranges; but it's horribly slow, what I actually need is: async(unzip_4MBData_chunks) | convert_into_records | chunk_by_key | fork(compress) | store_in_db convert_into_records and chunk_by_key don't need to know that what they are being fed is created asynchronously. store_in_db doesn't need to know that there are multiple compress-tasks feeding into this, fork returning a channel takes care of that. I could (and started a few times) to build my own data pipeline engine; it's not trivial and the whole infrastructure is just replicating a lot of what std::ranges already do (without interfering), that's why I am trying this approach. I have more cases where I have huge amounts of data being processed in multiple independent stages, it makes sense to have a framework for it. Conceptually I can resolve this, replace
with Horrible, I'll admit. It also blocks the thread; probably have to run this on its own thread. The overhead might actually be acceptable in my use cases, it just feels there should be a much nicer solution. |
|
So, if you want the "ranges pipeline syntax" using If you want your file reads to be truly async, you can do so using tmc-asio and setting ASIO_HAS_IO_URING and ASIO_DISABLE_EPOLL (assuming you are on Linux). Here are some ways to do this using the current facilities of TooManyCooks:
These all assume that you only want a single DB writer, but of course it's easy to scale that end up by adding more db_writer tasks. If you find that you are producing data too fast, and your DB writers can't keep up, you can use a tmc::semaphore to limit the number of in-flight chunks. |
|
I'm still adjusting to thinking asynchronously, I am realizing ranges/iterators just isn't suitable for async processing; Channels (or something like it) have to be the base for an async pipeline. I'll start off with the async that returns the channel. "transform" creates a task that iterates over it, forking a task for each item and writing those into a channel, returning that channel; instantly giving me concurrent stages and item-processing. And so on, treating "async" as the normal case instead of mixing async operations into a synchronous workflow. |
|
I've some ideas on how to provide facilities to make building this kind of pipeline easier, but an implementation will have to wait until after v1.3. The following is just a scratchpad: Method 1: Using channel to send messages and semaphore for backpressure. This makes it easy to have different levels of parallelism at each stage of the pipeline. Implementing with FileChunkReader myReader("filename.csv");
tmc::pipeline p{
tmc::pipeline_start{FileChunkReader::get_next_chunk, myReader},
tmc::pipeline_func{convert_into_records},
tmc::pipeline_func{chunk_by_key},
tmc::pipeline_func{compress}.with_concurrency(tmc::cpu_executor().thread_count()),
tmc::pipeline_end{db_writer}.with_concurrency(4),
}There could be a Each stage could be implemented as a number of parallel copies of: tmc::task<void> pipeline_stage(tmc::channel<Input> inChan, tmc::channel<Output> outChan, tmc::semaphore& inSem, tmc::semaphore& outSem, std::function<Output(Input)> process) {
auto input = co_await inChan.pull();
while (input.has_value()) {
inSem.release();
co_await outSem;
co_await outChan.push(process(input.value));
input = co_await inChan.pull();
}
}Rather than requiring a semaphore, it would be good to offer a bounded queue option (it would disable Also a specialization
Method 2: Directly invoking each stage - a replacement for #60. That issue originally was designed to let you attach a continuation to task after it was created, but this would need to happen before; letting you create a "pipeline template" which could be called any number of times with new inputs. This would not have any channels, but parallelism would be possible by a call to tmc::pipeline_direct p{
tmc::pipeline_task{load_file},
tmc::pipeline_func{file_to_chunks},
tmc::pipeline_func{decompress} // automatically concurrent if file_to_chunks returns an array
};
std::vector<DecompressedChunks> = co_await p.run("myfile.csv");This should desugar into a series of regular calls in a single coroutine: co_await [](std::string input){
auto a = co_await load_file(input);
auto b = file_to_chunks(a);
auto c = co_await tmc::spawn_func_many(
std::ranges::views::transform(b, [](Chunk& c) -> auto {
return [&c]() { return decompress(c); };
})
);
co_return c;
}("myfile.csv");Implementing this desugaring depends on the ability to emit Method 3: An ideal version would offers the benefits of both of the prior approaches:
A single-producer / single-consumer stage should be able to be pulled directly. Double / multiple buffering could be implemented like in https://gist.github.com/tzcnt/bfd1933b034da2761963adbb4025299c, but a new data structure would be needed in order to allow multiplexing M outputs -> N inputs efficiently. This could be a fixed size array with N slots. An atomic bitmap could be used to acquire/release slots. Would also need storage for a suspended consumer (or producer). To avoid needing to move data multiple times (like channel does), the producer could store the value directly in the array, and consumer could use the value by reference, then free it afterward. If done this way, the array should probably have 2N slots, and we would need multiple states in the bitmap - empty, data ready, data in use. This design is similar to channel, but should be more efficient. I wonder if any design elements of work contracts https://www.youtube.com/watch?v=oj-_vpZNMVw could be used to make this multiplexer more efficient? |
|
Method 2 is a no go for me. a) it requires the file to fit into Memory. b) it only starts processing once the complete file is loaded. Method 3 I don't fully understand, it seems to concentrate on reducing the overhead of using channels everywhere? Method 1 is what I am working on. Syntactic sugar aside (pipe operator, using variadic function/class...), backpressure and channel overhead seem to be the main issues there. You mentioned a bounded queue; indeed I had already thought about opening an issue about that :-) Maybe it is way to simplistic, but what I am thinking is there are 2 cases: a) the producer is faster than the consumer(s). What's the point of having multiple items waiting in a channel? They just take up memory, channel size = 1 should be sufficient. One could argue that different items can take different amounts of time to process so that sometimes consumption is faster and then there should be another item waiting already. At most there need to be cpu_count slots, in practice probably less. b) the consumer(s) is/are faster than the producer: the channel would never fill up to more than 1 item. A bounded_channel of static size N (implemented using a ring buffer?) that suspends on push would nicely solve the problem of the initial producer being too fast and should be more efficient than the unbounded channel. In this use case a "channel" of size 1 might even be sufficient and could be even more efficient, but that might be too specialized. A transformer that runs multiple items concurrently would indeed have to use a semaphore; the sub task uses acquire_scope and it cannot return it without being able to push to the bounded_channel. It almost feels too easy, but I think that solves the backpressure problem und negates the need for the complexity of method 3. Am I missing something? What I currently have (no sugar, proof of concept): a) The initial source b) transform This works, but the sub-tasks aren't started immediately. What I really want is
but that won't compile and as far as I can tell from documentation, it is impossible to move the result from fork() into a channel? I already had a very simple equivalent of a shared_future: so I can just use
It changes the type of the output channel and needs to be adapted as ForkedTask/fork to enable non-copyable items, but that would work. It's not really nice, maybe tmc offers a better way to pass around forked tasks? As we said, transform needs to use a semaphore internally. c) await d) the end of the pipeline. There is probably quite a bit of unnecessary overhead; for the heavy processing in my use case it hopefully doesn't matter. So far this approach seems promising to me: IMHO something like tmc::channel (but bounded) is absolutely needed to connect the different stages of a pipeline anyway and instead of having a rigid framework for a pipeline-object which has to be constructed in certain ways with special types (looking at std::ranges) , a "pipeline" is just couple of channels connected by functions, the only requirement for expansions being that the function takes a channel and maybe returns a channel. Two things I noticed: looks so ugly, but the far nicer crashes for me, no idea why. If that is a technical limitation, I think that should be in the documentation. I had known/read about problems with using lambda captures and coroutines; I assumed it was only about references, capture-by-value would be ok. Judging by all the crashes I got, using lambdas with coroutines you should never ever capture anything. Although not specific to TMC, a warning in the documentation would be nice. |
|
Don't mind my musings, you should definitely just use Method 1 (channel with backpressure) for now. Re: backpressure. Say your producer can produce an item in 1 second, and a consumer takes 5 seconds to process it.
This introduces a lot of extra latency. For this reason, I suggest having at least 1 channel capacity per consumer. Re: the performance of channel w/ semaphore vs. a bounded channel. I'll make a bounded channel eventually, but I discourage you from pushing tasks through the channel. Just push data through the channel, and spin up as many workers as you need for each parallel pipeline stage. This also means that you don't need separate Here is a complete implementation; please just use this for now... It's not perfect and I had to reach into the detail namespace at one place (this is more a fault of the stdlib - there's no standard way to get the type of an awaitable), so this has been a learning experience in trying to build something complicated on top of the library :) It has specializations for pipeline stages that are either functions or coroutines themselves. >> https://gist.github.com/tzcnt/2f50fb953dc9ee737571e422299b1964 << Thanks for sharing the I learned one other thing while implementing this - it was possible to convert a Yes, you can't capture anything at all in a lambda coroutine. Although this is a defect in the standard, I should probably add a section "Coroutine Basics" which covers this. One other thing - if you declare a function as a coroutine (return type is a task type), but the body of the function doesn't contain a |
|
One thing that may cause issues is that For now, if you really need "roughly FIFO" processing you can do it by not taking data from the input channel until after the output semaphore has been acquired. This cuts the performance on the benchmark significantly, but I was able to regain that performance by doubling the size of the semaphore (which allows for "double buffering"). See the changes to |
|
Glad I could contribute a bit with the syntax; I changed it in my code too, no problems. I guess I was still using lambda captures. "Coroutine Basics" would be a welcome addition to the docs; problems are hard to debug and coroutines have a lot of pitfalls. AI has absolutely no clue about it either and even less about TMC ;-) Are you sure the semaphore in your code does what it is supposed to do?. It does constrain the number of workers concurrently pulling the next value to 2 * workerCount, but there are only workerCount workers anyway. And the pulling itself is not the problem; there shouldn't be more than workerCount functions running at the same time (oversubscribing the CPU; using too much memory), that is already ensured by only having workerCount workers. And a channel shouldn't fill up unchecked when consumers are too slow (using too much memory), the semaphore doesn't constrain that. It would have to acquire on push and release on the pull. Except that the 2 operations happen in different stages which can't share a semaphore. That's where a bounded_channel would be awesome; not because of efficiency, but it makes it really easy to keep a fast producer in check ;-) The biggest problem for me is ordering, though. Some stages (compress, write_to_db) I don't care about the order; but "chunk_by_id" relies on the incoming data being exactly in the right order. Same in some other use cases. The easiest way I saw to ensure that was passing the tasks around, it seems to nicely model the intent. With a bounded_channel as target it would also solve the number of concurrently running tasks and a channel growing too large. I've come up with this for an ordered transform; borrowing your idea of spawning workerCount tasks; but instead of them pulling in random order (which can never result in the correct order on the other end) I have the stage pulling the items, enumerating them, using another channel to pass that to the workers which use an atomic_condvar to wait for their turn to push the result out: I'm not entirely happy with it; compared to just passing around a forked task in a channel it uses another channel and a notify_all for each item. And there is an obvious problem: a really slow work item would suspend all the other worker threads from pushing their results and processing the next one. If the next stage needs to process in order, that stage would suspend anyway, not a problem. If the order is not important, that is bad. So transform probably needs 2 different implementations, depending on if the ordering is important or not. |
|
The semaphore is a proxy for channel capacity, which allows us to create a bounded channel. This is what you said - "It would have to acquire on push and release on the pull." but your next statement was incorrect "the 2 operations happen in different stages which can't share a semaphore": The output semaphore of 1 stage and the input semaphore of the next stage refer to the same object, so that the producer acquires -> consumer releases pattern works. (the input semaphore is a pointer to the output semaphore of the previous stage) However I did realize that I had 1 bug in the implementation from last night, with the initial size of the semaphore. I was setting the size of the output semaphore based on the number of workers in the current stage, but I should have been setting the size of the input semaphore. I don't want to do gist-based development any more, so I've turned this into a proper example, and you can see the fix commit here: tzcnt/tmc-examples@899fad7 |
|
Re: ordering I finally get what you mean by forked task in a channel, and how it solves the problem of maintaining exact ordering. As usual there is some weird corner of the language to get this done. https://quuxplusone.github.io/blog/2018/05/17/super-elider-round-2/ lets you construct the non-movable object from the factory function into anything that emplaces. Such as template <class F> class with_result_of_t {
F&& fun;
public:
using T = decltype(std::declval<F&&>()());
explicit with_result_of_t(F&& f) : fun(std::forward<F>(f)) {}
operator T() { return fun(); }
};
template <class F> inline with_result_of_t<F> with_result_of(F&& f) {
return with_result_of_t<F>(std::forward<F>(f));
}
auto chan =
tmc::make_channel<std::unique_ptr<tmc::aw_spawn_fork<tmc::task<int>>>>();
auto in =
std::make_unique<tmc::aw_spawn_fork<tmc::task<int>>>(with_result_of([]() {
return tmc::spawn([]() -> tmc::task<int> { co_return 5; }()).fork();
}));
chan.post(std::move(in));
std::optional<std::unique_ptr<tmc::aw_spawn_fork<tmc::task<int>>>> out =
co_await chan.pull();
int a = co_await std::move(**out); |
|
No idea why I didn't see inSem and outSem; it was pretty much was I was looking for. Being able to move a started task is nice; I'll keep it in mind. The syntax isn't too nice and it's yet another type to handle... I took all the ideas and thoughts here and started implementing what best fits my use case. So far it's going well, thanks for all the help and explanations. |
|
Great! I'm going to convert this to a discussion. Feel free to ask any additional questions there. I have some ideas in mind as to how I can make this easier to implement, which I've created enhancement tickets for. I'll ping you on the discussion when those tickets have been closed.
|
Re: ordering
I finally get what you mean by forked task in a channel, and how it solves the problem of maintaining exact ordering.
As usual there is some weird corner of the language to get this done. https://quuxplusone.github.io/blog/2018/05/17/super-elider-round-2/ lets you construct the non-movable object from the factory function into anything that emplaces. Such as
std::optional::emplace, or withstd::make_unique. This will let you pass a forked task through a channel.