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
Make print() builtin synchronous #3028
Comments
While it's true changing from async to sync is backwards compatible, I'm a little worried about large maps overflowing the perg/ring buffer. In theory the user could be relying on the ability to print a huge map. We could always add some way for the user to opt in, but I also dislike these kinds of knobs in general. Seems like a tradeoff and I don't know the right answer atm. |
Tangentially related: rust and golang have a edition/version mechanisms for these kinds of subtle changes. Basically you tell the compiler which edition/version you want to build against, and compiler will maintain semantics of that version. Newer code, when you A mechanism like that could help us here. But it's probably overkill. Something to think about though. |
(Sorry for stream of consciousness) Maybe we could add an |
In general, I'm more in favour of adding new syntax instead of new bulitin functions as I think it tends to be more flexible. What about introducing a way to take references to maps and allowing them to be printed, e.g.:
It could also be useful in other scenarios, e.g. for double buffering:
|
print(@map)
is designed to print all entries in a map. It currently does this by passing a MapID reference to userspace, where bpftrace iterates over the map entries to print them out.The problem is that this makes
print(@map)
an asynchronous operation, which can produce unexpected results:Now that BPF has support for iterating over map elements in the kernel, we will be able to reimplement
print(@map)
as a synchronous operation by writing all the map keys+values to the ring buffer instead of a single MapID reference.I think this could be implemented with minimal new codegen once #3003 is merged - by rewriting the AST of any
print(@map)
call intofor ($kv : @map) { print($kv) }
. Some work will be needed to ensure the entries are still formatted and sorted correctly.The previous script should produce a more expected result in the future:
This also applies to some other builtin functions which operate on maps asynchronously:
clear()
,zero()
The text was updated successfully, but these errors were encountered: