Description
This is a feature proposal discussion for improving the batching api. As it stands right now if you want better read performance on hardware devices you must use the batcher, however doing so makes error handling quite a bit less convenient. The high level batcher does not handle partial or error read cases for individual items.
Steps to solve this:
The high level batching api depends on the read list and read iter api of memflow. These are also the main apis exposed to FFI at the moment. These functions do actually have callback functions already. These callbacks however only report the read address and success or failure.
With this current design it becomes pretty tricky to make an efficient way to track error cases on individual reads. To place the error result somewhere useful you have to maintain some sort of hashmap of target addresses to result states, which could add up in performance penalty over large quantities of reads.
To solve this I propose that the callbacks be extended with either a generic Optional<T> itemTag for the caller to assign as they see fit, a size_t idx for use in an array index, or an optional pointer to a result type (or int type on cpp) to be filled with the status code of that particular read entry. The index would be provided by the caller so that they can pass whatever identifiers they want on the callback for that particular entries callback.
Another option would be to have a result out pointer in the ReadDataRaw struct for status to write out to. This could even be made optional so that if someone does not care about status codes they are not required to provide this pointer.
After this extension it will be a much more convenient to design error handling in the high level batching for rust and in the ffi bindings to read iter.
Here is a code example to demonstrate how this could be helpful in ffi
// it would be nice to receive the errors in our little stack array here
int readStatus[sizeof(process_data::entries.item) / sizeof(process_data::entries.item[0])] = {};
// or into: process_data::entries.item[i].status_code
// c++ array implements iter which is important for the memflow c++ abi
[ReadDataRaw ops[](std::array<ReadDataRaw,)sizeof(process_data::entries.item) / sizeof(process_data::entries.item[0])> ops{};
// calculate the read ops
for (auto i = 0; i < process_data::num_entries && i < sizeof(process_data::entries.item) / sizeof(process_data::entries.item[0]); i++) {
auto tgt = process_data::entryList + 0x10 * i;
ops[i] = {
tgt,
tgt,
CSliceMut<uint8_t>((char*)process_data::entries.item[i].instance,
sizeof(uint64_t))
// if we had a way to pass result out ptr here then we could pass a reference per array entry
};
}
// wrap in the cglue iter type
CPPIterator read_iter(ops);
// cglue callback wrap
OpaqueCallback<ReadData> success_cb(
[](ReadData data /* if we had an idx here we could map the result to our error map*/) {
printf("success_cb %llx : %llx\n", data._0, data._1);
return true;
});
OpaqueCallback<ReadData> error_cb(
[](ReadData data) {
printf("error_cb %llx : %llx\n", data._0, data._1);
return true;
});
ReadRawMemOps readOps{
read_iter,
& success_cb,
& error_cb
};
g_memflow_state.process.read_raw_iter(
readOps
);Thoughts?
Heres the the original suggestion from discord as well for parity:
what do people think of the idea of extending the scatter read calback api with an optional "key" or index? Would be remarkably useful and efficient for running a batch on an indexed table of operations. Basically: send out batch requests for your idx 1..1000 or whatnot, then callback comes back with the success or failure tied to an index so you can mark that entry with the status code. Would be super helpful for error handling of partial or failed reads when using the batch read system.
Without an optional "key" parameter you would need to maintain a hashmap of read addresses to accomplish this as far as i know; Which is a a decent bit slower when the number of read ops is substantial
ko1N — 11:37 AM
i think this would just require a rewrite of the batcher itself, the read_iter functions already provide a callback for when reads fail
so no need to necessarily change the read iter stuff
and i agree, having no feedback on the reads could be improved with the batcher api
segfault — 11:46 AM
yea they do, my proposal is essentially to add one more argument to the end of the callback. Optional<T> itemTag or size_t idx. This would be provided by the initial read request by the caller, and can be added while completely leaving the current batcher api unchanged for now
This would allow for the batcher api to finally get the read feedback feature in the future 🙂
another alternative is to extend the write callback address with a wrapper for error out pointer. So it would be a tuple of result out and value out