Skip to content
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

Rust support #44

Open
OtaK opened this issue Jul 13, 2018 · 11 comments
Open

Rust support #44

OtaK opened this issue Jul 13, 2018 · 11 comments

Comments

@OtaK
Copy link

OtaK commented Jul 13, 2018

Hello,

I wanted to ask about rust support.

Considering how such code is usually generated with macros, should we just create a crate that just implements the protocol/ser/deser logic?
Also, conforming with Serde to get automatic ser/deser of rust struct would be a very nice addition as well.

Cheers,

Mathieu

@pascaldekloe
Copy link
Owner

Hi Mathieu,

Can you elaborate what you mean by the "protocol/ser/deser logic", maybe with a link? To a Rust noob like me it sounds like you suggest to (re)use the interface from Protocol Buffers?

@OtaK
Copy link
Author

OtaK commented Jul 14, 2018

What I meant by protocol is the Colfer data format, sorry.

As I'd like to use Colfer in my environment (which is hybrid Rust/JS/Go/C), would you like me to just go ahead and implement something in idiomatic rust?

The only problem I see with idiomatic Rust is that it'll drift away from the code generation logic you implemented.

@pascaldekloe
Copy link
Owner

Oh whauw! :D I would be extremely pleased with any help on the subject. Please feel free to make another API if you see room for improvement. The main reason for the existing design is to minimise memory allocation. It should be at least possible to reuse the buffers and structs. Maybe you don't need it because of Rust's stack allocation?

Maybe I should do the compiler/template work so you can fully focus on the implementation and I learn a bit about the language? Could you write an implementation of ./testdata/test.colf in say ./rust/gen/Colfer.ru. It's OK if you skip a few fields/datatypes. The compiler can report errors for unsupported mappings where needed.

Not sure whether you are on a schedule? I'm on holliday until the 20th. The compler follow-up should be done within 2 days.

@OtaK
Copy link
Author

OtaK commented Jul 14, 2018

Glad you're hyped with the proposal!
Colfer is amazing, we're already using it on an in-progress IoT implementation as the chosen binary format over ZeroMQ because it's insanely fast and small. I can only say you did some amazing work on Colfer.

And I'm aiming to use it in our backend architecture as well (distributed with serialized binary messages passed over a message broker) because well, even if it's not a bottleneck in backend development, it's still a welcome optimization, and our backend is part Rust, part Node.
Which is why I kinda want to go as fast as possible on getting my hands on a Rust version, even if I'd have to write it myself.

I'd like to keep your API to ease maintenance that's for sure, same method everywhere, but I think that for Rust, to keep the project quite idiomatic with what's possible, we have two choices:

  • (suboptimal, but more in line with the existing) Dynamically generate Rust code and writing it out on a file as you did; only problem is that developers kinda lose the advantage of building the whole project with cargo, they'd have to build the .colf scheme first and then proceed to build their own project.

  • (more idiomatic Rust) A better choice in my opinion would be to take advantage of Rust procedural macros. We'd be able to dynamically "enhance" a struct by adding the colfer methods to it at build time. It's very neat and for the user, optin-in to Colfer Ser/Deser methods would be as simple as adding #[derive(Colf)] on top of their struct.

@pascaldekloe
Copy link
Owner

Those macro features look interesting. One downside of using Rust structs as definition types is maintenance when used with more than one language; how to keep the definitions in sync?

The cargo documentation mentions support for code generators explicitly. Would it not be easier to launch colf(1) with a build.rs? Maybe even have those procedural macros print Colfer schemas if you please?

I leave the decision up to you. Don't worry about being in line of what's already there. We need the most elegant solution.

@OtaK
Copy link
Author

OtaK commented Jul 15, 2018

I've thought about the build.rs solution (I'm actually using it in a project to generate code from a XML definition file of Modbus registers) and it has several issues:

  • For people who are already using a build.rs file, it means editing theirs.
  • For people who don't have one, it means setting it up, which is some overhead but acceptable.
  • The biggest "no-go" point for me is the non-elegance of having to include such generated files like this: include!(concat!(env!("OUT_DIR"), "/your_output_file.rs"));

About the definitions in sync issue, we could have both options:

  • #[derive(Colf)] as the prime means of automatically serialization/deserialization logic
  • a userland macro (eg: colf_derive_from_file!("path_to_file.colf");) which would import the colf file, declare the struct from it and automatically add the above #[derive(Colf)] to it, implementing the logic along with it.

@pascaldekloe
Copy link
Owner

Two more days of holliday and I'm back home coding. How's the implementation going? Do you have a public repository somewhere? =)

@pascaldekloe
Copy link
Owner

Please note that you need some way to lock the compiler version for consistent builds.

As noted in the README, I recommend to put the generated code in the SCM. This also eliminates include!(concat!(env!("OUT_DIR"), "/your_output_file.rs"));

@OtaK
Copy link
Author

OtaK commented Jul 19, 2018

Hey,

I've been super busy (on a schedule) over the last few days, and couldn't do anything more than I wrote last saturday. I've been writing a rust version of generated code (by hand) just to check the implementation of serialization/deserialization. Pretty sure most of it is plain wrong/broken since I've wrote it in only one run without testing anything.

The repo is located here: https://github.com/YellowInnovation/colfer-rs

I think the option of using build.rs (and include!(concat!(env!("OUT_DIR")...))) is really not that good in general, I've looked into it and I honestly think the solution I've described at the end of my previous comment is kinda the best possible in terms of idiomatic rust.

@pascaldekloe
Copy link
Owner

Got some time to spare later this week. Can you please declare your project public domain [CC0] so I can use your implementation. If you write a unit test with the test set for your types.rs then we're golden.

Once you manage the procedural macro's then I'll be more than happy to link to your project from here.

I see that you use a switch-loop for the fields with unmarshal. As the field indices are required to count up I found that ifs [conditional jumps] work faster unless you do a function lookup with a table for the 256 byte values.

@OtaK
Copy link
Author

OtaK commented Jul 26, 2018

Pattern matching in rust internally works as a scoped function lookup AFAIK so it's not a "switch" per-se as you would understand it in other languages. The semantics of it are close to a switch statement but much more powerful and with better performance compared to a bunch of ifs.

My implementation is pretty much broken right now, I've noticed many problems with it (and I think I went to a completely wrong route of implementing it, I tried porting without thinking and actually there's a much better way to write it in Rust), I'd personally advise against even looking at it lmao.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants