search

mozilla / uniffi-rs

a multi-language bindings generator for rust
https://mozilla.github.io/uniffi-rs/
Mozilla Public License 2.0
2.47k stars 210 forks source link

Use C structs/tagged unions for Records/Enums #2156

Open bendk opened 1 week ago

bendk commented 1 week ago

Currently these types are always serialized into a RustBuffer which has been long believed to be inefficient (Issue 244, ADR-002). We should explore passing these as repr(C) structs and tagged unions to avoid the overhead of serialization and allocating a buffer.

These could either be passed across the FFI as a pointer/reference or as the struct directly. Some of the recent changes could help here, for example we now have both FfiType::Struct and FfiType::Reference.

Any change here should be accompanied by a benchmark that measures performance changes.

bendk commented 1 week ago

Any change here should be accompanied by a benchmark that measures performance changes.

Interestingly, the very first benchmark tackled an issue related to this: the callback interface system that passed a RustBuffer struct across the FFI was slow on Kotlin and Python and performance was improved by passing data and len arguments rather than a single RustBuffer argument. After doing some research on it, I now believe this was caused by the amd64 calling conventions. When that PR landed, RustBuffer contained a data pointer and 2 u32 fields for a total of 2 words. By convention, callers pack that data into 2 registers. This is normally an optimization, but when the data needs to flow through libffi, JNA, and then be unpacked by Python/Kotlin I believe it actually slowed things down.

I'm not sure what we should do about this: I might be misdiagnosing the issue, maybe it's a non-issue that only happens with very particular structs, or maybe we should prefer passing structs using pointers to avoid the performance hit.

badboy commented 1 week ago

In the past (before switching to UniFFI) Glean used c structs/tagged enums to pass some data and we encountered at least one edge case/bug in JNA causing data corruption along the way. That's probably (hopefully?) fixed now, if we go this route we need to have extensive testing to ensure it works.

fwiw, I did have this idea for UniFFI a long while ago as well, just never acted on it.