-
Notifications
You must be signed in to change notification settings - Fork 225
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
Support for encoding/decoding external structs #200
Comments
I spent some time thinking about this as well, but did not yet arrive at a good solution yet. We use feature flags to work around that (disable compiling EDIT: serde-transcode could be useful to avoid serializing into an intermediate format here. |
@davydog187, @evnu can one of you provide a little bit more detail into what your expectations are for this? Is this so that you don't have to wrap external types in a new type? |
Not exactly. I sometimes encounter the following scenario. Imagine a // crate compute
pub mod compute {
#[derive(Debug)]
pub struct A {
a: i32,
}
impl A {
pub fn new(a: i32) -> Self {
Self { a: a }
}
}
}
// crate compute_nif
pub mod compute_nif {
pub fn compute_an_a<'a>(env: Env<'a>) -> NifResult {
let a = compute::A::new(10);
a.encode(env);
}
}
// crate compute_cli
pub mod compute_cli {
fn main() {
let a = compute::A::new(10);
println!("a: {}", a);
}
} So, it would be nice if @davydog187 I hope that example also covers what you are asking for, I did not intend to hijack this issue. EDIT: I forgot to mention: If the remote type is wrapped in a newtype, the remote type still needs to implement |
Let's image than I have /// crate_rust
pub struct A {
val: String
} Now, if I want to use |
@evnu I think we're on the exact same page here 😆 |
This also brings up another issue, as far as I can tell, tuple structs and newtypes are not encodeable/decodeable |
I see. Yeah, rust's orphan rules make this pretty interesting. I'm not sure exactly how to go about supporting this. But if |
Ah, I was not aware serde was doing it that way. Don't really like it, but it solves the problem at least. So as far as I can see there are two alternatives here:
Any opinions? |
It seems like if we supported encoding/decoding with serde's traits, we would still have the issue of not all types deriving Does that sound right? If so, we would probably need to support both? |
If we supported the serde traits, it would enable the user to use serde's mechanism for remote structs. But again, that's not completely optimal either. We should probably support something similar to what they are doing in our macros. |
Do have something in mind for that? As far as I understand, it is not possible to retrieve the AST for a remote type. Then, the remote struct definition must be explicitly copied which again results in code duplication. |
@evnu I would love to avoid having to do that, but as far as I understand there really isn't any alternative? I suspect serde would have used a better solution if it existed |
Is the idea here to take a Rust struct and serialize it into erlang term format. Then use |
@elbow-jason The idea was to encode/decode the serde traits directly from the in memory term representation and back, much like we do currently with our own encoder/decoder traits. This should hopefully have less of an overhead than going through the binary format. |
@hansihe wrote a library pertaining to the issue: serde_rustler provides a Serializer and Deserializer for directly encoding/decoding Rust types to/from Elixir terms. Encoding and decoding benchmarks look promising (like, insanely, unbelievably promising), and given that I haven't published many Rust crates or Elixir packages, I'd greatly appreciate feedback on reconfiguring the benchmarks, improving the tests, changing the serialization/deserialization behavior, or just adding to/improving the API. Hope y'all enjoy! |
@sunny-g Great work! Seeing as this would probably be useful for quite a few people I think it would make sense to at last add it to the Readme of rustler? I looked through it briefly, and it looked pretty good from what I could tell. I'll hopefully find some time to look through it more thoroughly soon. |
@sunny-g I stumbled across your repo the other day. Great work! Should this be folded into rustler or should it stay as a separate library? My worry is that the fragmentation makes it less accessible |
PR to add link to readme here: #224. @davydog187 I'm not opposed to it, but I'm still in the process of debugging some performance issues and experimenting with additional features (e.g. a |
@sunny-g were you able to make progress? |
Just to chime in here. I’d love to see @sunny-g excellent work! |
@evnu never got around to it 😦 @scrogson Appreciate that! I've been coming around to this idea as well - To start, I think it could be done with as little as an |
@davydog187 @hansihe I'm not really using Elixir much anymore (my foray into If someone wants to take this task, I think the easiest and most simplifying (though breaking) changes would be what I mentioned in my previous comment here, i.e.
If / however y'all want to do this is fine with me, and I can help anyone who wants to pick up the task. |
I started integrating this into rustler over the weekend. |
Howdy! I've ended up updating serde_rustler using my branch to point to the latest versions of Rustler. Are there still plans to roll this in here (as a feature flag, I'm guessing)? I'm writing a library using rustler for req_snowflake, and Snowflake supports two different response types: JSON & Arrow. I'm using arrow2 Rust library for parsing these Arrow IPC Streams (they're files Snowflkae sends back as gzipped), and I'm honestly blown away by how fast the NIF is. Initial benchmarks show promising results, and we can even create Elixir types from Rust as we know the types from Arrow. I've got little experience with Rust, just having used it to create a few small CLI apps here and there, so would love feedback on when the library is ready. |
Hi @scrogson and friends 👋🏼 I was recently in contact with @sunny-g and we briefly discussed the future of The future is in your hands: if you still believe that integration is the best path forward, please let me know how we can help. If another path is better, let's see what we can do. 👍🏼 |
@aj-foster Integrating |
@evnu @aj-foster works for me! Depending on the usage of the existing macros, I would still recommend what I mentioned here; otherwise @aj-foster and I can spearhead a simple PR to just merge |
Hi @evnu, I'm happy to prepare a pull request. I'm curious how you would like to approach this from an ergonomic perspective. We can take a relatively light approach (adding the package here, still segmented from the main code), or a more integrated approach like Sunny mentioned above. Please feel free to spell out what you're imagining like I'm a small child — I'll need guidance throughout this process. 🙂 |
@aj-foster good question on the steps. Lets keep it simple for now and start with vendoring Afterwards, we will need to discuss how to integrate So, I propose these steps:
Does that make sense? CC @rusterlium/core |
@evnu |
@scrogson of course, I forgot about that part, thanks for pointing that out. So instead we would need to check if we can deprecate |
First of all, thank you for all the hard work on rustler!
We have been making extensive use of Rustler as a means to run simulations and cpu bound computations from Elixir.
We keep running into the problem of wanting to write Rust only libraries, then import them from an outer crate that exposes the library to Elixir. The issue is that Rustler only supports compile time encoding/decoding, which means that we either need to write an adapter and duplicate our data structures, or find some other means.
It would be wonderful if Rustler provided a mechanism to Encode/Decode external data structures so that Rustler doesn't leak into our lower level dependencies.
We are very interested in writing this feature for Rustler, but I first wanted to see if there has been any thought on this front in the past.
Thank you!
The text was updated successfully, but these errors were encountered: