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

Port FlatBuffers to Rust #4898

Merged
merged 37 commits into from Sep 3, 2018
Merged

Conversation

@rw
Copy link
Collaborator

@rw rw commented Aug 29, 2018

This is a port of FlatBuffers to native Rust. It provides code generation and a runtime library derived from the C++ implementation. It utilizes the Rust type system to provide safe and fast traversal of FlatBuffers data.

There are 188 tests, including many fuzz tests of roundtrips for various serialization scenarios. Initial benchmarks indicate that the canonical example payload can be written in ~700ns, and traversed in ~100ns.

Rustaceans may be interested in the Follow,Push, and SafeSliceAccess traits. These traits lift traversals, reads, writes, and slice accesses into the type system, providing abstraction with no runtime penalty.

@rw rw force-pushed the 2018-08-28--rw-flatbuffers-rust branch from c855cd4 to e92a41e Aug 29, 2018
@ry
Copy link

@ry ry commented Aug 29, 2018

Awesome work @rw - thank you! We're already using your patch in our project and it's working great.

@rw rw force-pushed the 2018-08-28--rw-flatbuffers-rust branch from e92a41e to 4722510 Aug 29, 2018
@aardappel aardappel self-requested a review Aug 29, 2018
@rw rw force-pushed the 2018-08-28--rw-flatbuffers-rust branch 9 times, most recently from dff1ac4 to de40027 Aug 29, 2018
@rw rw self-assigned this Aug 29, 2018
@rw rw force-pushed the 2018-08-28--rw-flatbuffers-rust branch 10 times, most recently from 5c1afc2 to 21fefe9 Aug 30, 2018
@rw rw force-pushed the 2018-08-28--rw-flatbuffers-rust branch from 21fefe9 to 93f9162 Aug 30, 2018
@phayes
Copy link

@phayes phayes commented Aug 30, 2018

Should the fie tests/rust_usage_test/test_bench_output.txt have been included in this PR?

@rw
Copy link
Collaborator Author

@rw rw commented Aug 30, 2018

@phayes I included it in the PR for people who don't want to run the code but still want to see the output of tests passing.

Copy link
Collaborator

@aardappel aardappel left a comment

WOW! This is some amazing effort, the most comprehensive language port yet!

I'm missing Rust specific changes to Tutorial.md, and a Rust sample in samples. Here is a sample checklist to see if you've touched all typical language port files: 4898809

CMakeLists.txt Show resolved Hide resolved
The generated accessor functions access fields over offsets, which is
very quick. These offsets are not verified at run-time, so a malformed
buffer could cause a program to crash by accessing random memory. (We try to
prevent this in Rust by using safe slice accesses, instead of unsafe pointer
Copy link
Collaborator

@aardappel aardappel Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this phrase is a bit weird.. what exactly happens if an offset in the buffer that points outside of the buffer is turned into a slice, and then accessed? How is that different from a raw pointer?

data from the network that can potentially have been modified by an
attacker, this is undesirable.

The C++ port provides a buffer verifier, but, at this time, Rust does not.
Copy link
Collaborator

@aardappel aardappel Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Rust may provide this in a future version" ?

/// the default, then this is a no-op.
#[inline]
pub fn push_slot<X: Push + PartialEq>(&mut self, slotoff: VOffsetT, x: X, default: X) {
self.assert_nested("push_slot must be called after start_table");
Copy link
Collaborator

@aardappel aardappel Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that users of the generated API likely aren't calling push_slot themselves, this message may be confusing? In fact, it may be easier to just have one message for nesting violations.

}

#[inline]
fn fill(&mut self, zero_pad_bytes: usize) {
Copy link
Collaborator

@aardappel aardappel Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function doesn't appear to do anything?

@@ -14,8 +14,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.

../flatc --cpp --java --csharp --dart --go --binary --lobster --lua --python --js --ts --php --grpc --gen-mutable --reflect-names --gen-object-api --no-includes --cpp-ptr-type flatbuffers::unique_ptr --no-fb-import -I include_test monster_test.fbs monsterdata_test.json
../flatc --cpp --java --csharp --dart --go --binary --lobster --lua --python --js --ts --php --gen-mutable --reflect-names --no-fb-import --cpp-ptr-type flatbuffers::unique_ptr -o namespace_test namespace_test/namespace_test1.fbs namespace_test/namespace_test2.fbs
../flatc --cpp --java --csharp --dart --go --binary --lobster --lua --python --js --ts --php --rust --grpc --gen-mutable --reflect-names --gen-object-api --no-includes --cpp-ptr-type flatbuffers::unique_ptr --no-fb-import -I include_test monster_test.fbs monsterdata_test.json
Copy link
Collaborator

@aardappel aardappel Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please change .bat as well

Copy link
Collaborator Author

@rw rw Sep 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

impl<'a> flatbuffers::Follow<'a> for Monster<'a> {
type Inner = Monster<'a>;
#[inline]
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
Copy link
Collaborator

@aardappel aardappel Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be _follow if there is a possibility of a clash with generated fields?

Copy link
Collaborator Author

@rw rw Sep 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good eye! This is actually okay because it's a trait method, so unless users are importing Push, they cannot access it. Furthermore, I added the trait methods we use to the reserved keywords list, so a field called follow would be generated as follow_.

@@ -0,0 +1,184 @@
running 170 tests
Copy link
Collaborator

@aardappel aardappel Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems weird to include this in the repo?

Copy link
Collaborator Author

@rw rw Sep 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed it

let test4 = builder.create_vector_direct(&[my_game::example::Test::new(10, 20),
my_game::example::Test::new(30, 40)]);
let pos = my_game::example::Vec3::new(1.0, 2.0, 3.0, 3.0, my_game::example::Color::Green, &my_game::example::Test::new(5i16, 6i8));
let args = my_game::example::MonsterArgs{
Copy link
Collaborator

@aardappel aardappel Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inline in the create call?

Copy link
Collaborator Author

@rw rw Sep 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment explaining why we can't do this (the Args struct takes a reference to a Vec3 so we have to make sure it lives long enough).

None => { return Err("bad m.testarrayofstring"); }
Some(x) => { x }
};
if testarrayofstring.len() != 2 { return Err("bad monster.testarrayofstring len"); }
Copy link
Collaborator

@aardappel aardappel Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should bad fieldname be generated with a macro?

Copy link
Collaborator

@aardappel aardappel Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or just use assert_eq ?

Copy link
Collaborator Author

@rw rw Sep 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrote a macro for this.

@rw
Copy link
Collaborator Author

@rw rw commented Sep 2, 2018

@aardappel Thanks for the feedback, I incorporated it all. Also, I resolved an alignment issue that I didn't catch before, and made the Push trait more useful.

@aardappel aardappel merged commit 0758c1e into google:master Sep 3, 2018
3 checks passed
@aardappel
Copy link
Collaborator

@aardappel aardappel commented Sep 3, 2018

Thanks! Amazing work!

rw added a commit that referenced this issue Sep 3, 2018
This is a port of FlatBuffers to Rust. It provides code generation and a
runtime library derived from the C++ implementation. It utilizes the
Rust type system to provide safe and fast traversal of FlatBuffers data.

There are 188 tests, including many fuzz tests of roundtrips for various
serialization scenarios. Initial benchmarks indicate that the canonical
example payload can be written in ~700ns, and traversed in ~100ns.

Rustaceans may be interested in the Follow,Push, and SafeSliceAccess
traits. These traits lift traversals, reads, writes, and slice accesses
into the type system, providing abstraction with no runtime penalty.
rw added a commit that referenced this issue Sep 3, 2018
This is a port of FlatBuffers to Rust. It provides code generation and a
runtime library derived from the C++ implementation. It utilizes the
Rust type system to provide safe and fast traversal of FlatBuffers data.

There are 188 tests, including many fuzz tests of roundtrips for various
serialization scenarios. Initial benchmarks indicate that the canonical
example payload can be written in ~700ns, and traversed in ~100ns.

Rustaceans may be interested in the Follow, Push, and SafeSliceAccess
traits. These traits lift traversals, reads, writes, and slice accesses
into the type system, providing abstraction with no runtime penalty.
@binary132
Copy link

@binary132 binary132 commented Sep 3, 2018

👏 👏 👏

termoshtt pushed a commit to termoshtt/rust-flatbuffers that referenced this issue Dec 14, 2018
This is a port of FlatBuffers to Rust. It provides code generation and a
runtime library derived from the C++ implementation. It utilizes the
Rust type system to provide safe and fast traversal of FlatBuffers data.

There are 188 tests, including many fuzz tests of roundtrips for various
serialization scenarios. Initial benchmarks indicate that the canonical
example payload can be written in ~700ns, and traversed in ~100ns.

Rustaceans may be interested in the Follow, Push, and SafeSliceAccess
traits. These traits lift traversals, reads, writes, and slice accesses
into the type system, providing abstraction with no runtime penalty.
zchee added a commit to zchee/flatbuffers that referenced this issue Feb 14, 2019
This is a port of FlatBuffers to Rust. It provides code generation and a
runtime library derived from the C++ implementation. It utilizes the
Rust type system to provide safe and fast traversal of FlatBuffers data.

There are 188 tests, including many fuzz tests of roundtrips for various
serialization scenarios. Initial benchmarks indicate that the canonical
example payload can be written in ~700ns, and traversed in ~100ns.

Rustaceans may be interested in the Follow, Push, and SafeSliceAccess
traits. These traits lift traversals, reads, writes, and slice accesses
into the type system, providing abstraction with no runtime penalty.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants