Refactor log macro for readability #683
Conversation
✅ Deploy Preview for aya-rs-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
This happens to fix the miscompilation that occurs when bpf-linker is moved to LLVM's new pass manager. A later commit will avoid the miscompilation more convincingly.
|
Do you know what the miscompilation looks like and why this fixes it? |
|
The hypothesis is lining thresholds change under the new PM, and this rearrangement gets this inlined with the new PM. The root cause is that the logging functions should take self by reference to avoid passing lots of parameters and triggering miscompilation. I'll do that in this PR. |
| } | ||
| } | ||
| match unsafe { &mut ::aya_log_ebpf::AYA_LOG_BUF }.get_ptr_mut(0).and_then(|ptr| unsafe { ptr.as_mut() }) { | ||
| None => {}, |
There was a problem hiding this comment.
isn't this if let? Why doesn't clippy complain about this?
|
If you're going to rework the WriteToBuf impls or whtever it's called, it would be good to actually remove the inlining from aya-log. It's common to have several log calls and aya-log currently consumes a big part of the insn budget because of all the inlining. |
I think we should #[inline(never)] TagLenValue::write, since llvm isn't very smart at not over inlining monomorphized code. But it doesn't necessarily have to be in this PR, up to you. |
|
And also the inline on writing the record header needs to go too |
|
Yeah, I agree. I started pulling on this thread and it's getting a bit involved; I'm going to merge this as-is now and revisit when we have working debuginfo which will make figuring out the verifier issues I'm chasing now easier. |
This happens to fix the miscompilation that occurs when bpf-linker is
moved to LLVM's new pass manager. A later commit will avoid the
miscompilation more convincingly.