Support CStr

[conradludgate]

I noticed that passing &str or String requires converting to a &[u8] where you then push a nul byte. It would be nice to support CStr as well to avoid this alloc.

Additionally, instead of doing

// args.1: String
let arg_1 = [(args.1.as_ref() as &str).as_bytes(), &[0_u8]].concat();

it would likely be better instead to do

let arg_1 = {
    let mut bytes = args.1.into_bytes();
    bytes.push(0);
    bytes
};

[ahl]

I noticed that passing &str or String requires converting to a &[u8] where you then push a nul byte. It would be nice to support CStr as well to avoid this alloc.

It would be nice to support Cstr, but note that the allocation only occurs when the probe is enabled; the cost of allocation is typically pretty small compared with the cost of the probe execution. We would welcome a PR for Cstr support.

Additionally, instead of doing

// args.1: String
let arg_1 = [(args.1.as_ref() as &str).as_bytes(), &[0_u8]].concat();
it would likely be better instead to do

let arg_1 = {
let mut bytes = args.1.into_bytes();
bytes.push(0);
bytes
};

It would be interesting to know if those produce different asm; even if they did I would be surprised if the difference wasn't in the noise.

[bnaecker]

Could you say a bit more about what you want to do? Do you already have a CStr or CString, and you want to pass it as-is without converting it to a &str or String? Or do you have a Rust string type, and you want to pass it as a C-compatible string?

[conradludgate]

Could you say a bit more about what you want to do? Do you already have a CStr or CString, and you want to pass it as-is without converting it to a &str or String? Or do you have a Rust string type, and you want to pass it as a C-compatible string?

Strictly speaking I don't need it right now. I was looking into how best to use usdt and I noticed the forced allocation which just called out to me to be fixed.

My main concern was actually that an already allocated string is unnecessarily re-allocated just to add the null byte. Looking at the examples in the repo where format!() is used for the argument - it felt trivial to me that the allocation should be reused and the nul byte simply appended to the end.

[ahl]

Strictly speaking I don't need it right now. I was looking into how best to use usdt and I noticed the forced allocation which just called out to me to be fixed.

That may reflect a misunderstanding of its use: the allocation only occurs if the probe is enabled.

Alternatively: to evaluate a potential performance improvement, one needs a reasonable benchmark demonstrating that the change makes that benchmark faster.

[bnaecker]

I was in the middle of a response that echoes what @ahl just said. The allocation only happens when the probe is enabled, not all the time. And in that case, you have to allocate something to make a C-compatible string -- either the probe closure takes the argument by value and null-terminates it, or makes a copy / concatenates a zero, the way we have it today.

That was why I also asked about your use case. If you already have a C-compatible string of some kind (CStr for example), I'd recommend passing that as a *const u8 in the probe definition, and then doing my_cstr.as_ptr() in the closure that generates the arguments.

[conradludgate]

That may reflect a misunderstanding of its use: the allocation only occurs if the probe is enabled.

There's no misunderstanding here. I'm just trying to help improve performance for the case where there is a probe attached.

[ahl]

Thanks. The way we typically do performance work is to construct a test, measure the performance, make a change, and demonstrate that the change has a measurable, positive impact.