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

Streaming flatbuffers? #3898

Closed
bog-dan-ro opened this issue Jun 3, 2016 · 16 comments

Comments

@bog-dan-ro
Copy link
Contributor

commented Jun 3, 2016

With ProtocolBuffers is very easy to serialize, stream multiple "messages".
Basically you just need to write one after another, same when you read the data, you don't need to know the length of every message.
It will be nice is flatbuffers (or the verifier) will tell how much data it's using from the buffer, this way we know where the next message starts.

@gwvo

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2016

Yes, a FlatBuffer doesn't encode length, and we can't add that now.

What we can do is introduce the concept of a (uint32_t) length-prefixed buffer, intended to be chained. This is of course simple to do, but it be good if there was standard functionality for it in FlatBuffers, so people don't have to reinvent the wheel. It could be added as a flag to Finish, such that it can take care of ensuring the buffer is properly aligned.

Are you able to make a PR?

@bog-dan-ro

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2016

I thought the Verifier knows (or at least can "guess") the size of the message...

I can try to implement and make a PR (PR stands for Pull Request, right), but probably it will take much more than for you :), I'm still not very familiar with flatbuffers internals.

@gwvo

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2016

Yes, the verifier could tell you the upper bound within which all data sits, bit it is slightly messy (it would need to know what the buffer is aligned to, which there is no way to tell). Without a guaranteed exact number, there's no way to reliably skip to the next message. And besides that it is an expensive way to compute the size, if you didn't need verification.

Yes, a PR. I typically suggest this, because me getting to have time to implement it may take a while, so if you're in a hurry..

@gwvo

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2016

Actually, alignment padding for the last element in a buffer only happens for strings, vectors with 16bit elements or less, so it IS possible to reliably figure out the buffer size this way. Basically it needs to account for all cases where flatbuffers.h calls PreAlign(). Even simpler, since a FlatBuffer is always at least 4-byte aligned, it can blindly increase the upper bound to be 4-byte aligned when it isn't, and this should be accurate. Any bigger elements (e.g. doubles, SIMD types) always produce a reliable upper bound.

I'll make a note of that, this would be nice functionality to add to the verifier. It can even be controlled with a #define such that it doesn't slow down people that don't need it.

@bog-dan-ro

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2016

Well, in principle people will (should) use the verifier in such cases, because first and foremost you need to know if there is garbage in the stream which will crash your application, at the end of the verification if you'll get also the size of the stream, that is the bonus ;-).

I'll try to implement it myself, any more tips about how to do it will be highly appreciated ;-) !

@bog-dan-ro

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2016

Can you please check if I got it right #3905 ? :)
It seems way too easy :)

bog-dan-ro added a commit to bog-dan-ro/flatbuffers that referenced this issue Jun 8, 2016
bog-dan-ro added a commit to bog-dan-ro/flatbuffers that referenced this issue Jun 9, 2016
bog-dan-ro added a commit to bog-dan-ro/flatbuffers that referenced this issue Jun 9, 2016
bog-dan-ro added a commit to bog-dan-ro/flatbuffers that referenced this issue Jun 9, 2016
bog-dan-ro added a commit to bog-dan-ro/flatbuffers that referenced this issue Jun 17, 2016
bog-dan-ro added a commit to bog-dan-ro/flatbuffers that referenced this issue Jul 13, 2016
bog-dan-ro added a commit to bog-dan-ro/flatbuffers that referenced this issue Jul 13, 2016
bog-dan-ro added a commit to bog-dan-ro/flatbuffers that referenced this issue Jul 14, 2016
@gwvo gwvo closed this in #3905 Jul 14, 2016
TGIshib added a commit to TGIshib/flatbuffers that referenced this issue Aug 3, 2016
Update idl_gen_fbs.cpp

Update idl_parser.cpp

Update idl_gen_general.cpp

Update idl_gen_general.cpp

fixed initialization of member var for old make (hopefully)

fix missing space (clang format)

same fix for general code generator

Fix for issue google#3922

Also, clean up redundant ';' at end of java classes.

Fixed operator++. Added CreateXXX for vector types.

Fixed operator++. Added CreateXXX for vector types.

New CreateXXX (with vectors and strings) calls old CreateXXX (with offsets).

Added function GenSimpleParam. Tests added.

Fixed spaces. Removed redundant == nullptr. Vectors pointers made const.

Rename CONTRIBUTING to CONTRIBUTING.md

Don't crash if str is null

Is useful especially when we want to create a string from another message string that might be null.

Update test.cpp

Update test.cpp

Update test.cpp

Update test.cpp

Verifier computes the buffersize, useful for streaming

Close google#3898

Verify everything in one shot

Added optional object based API for C++.

Change-Id: If927f3ea3fb3723088fa287f24bdd1ad43c8d1d1
Tested: on Linux.

Fix link to CONTRIBUTING

clangFormating base class

clang formating cpp code generator and add missing generated classes

Create a maven like project structure for java development. Make it OSGi compliant. Generate the flatbuffers code for testing (example).

Java developer are mostly comfortable with maven project structure. One one the main concept behind maven is convention. If you follow the maven project convention then your development team will get more effective as they now this project structure and can easily find the production code versus the test code.
 In this pull request I have structured the java project around 2 main parts:
  * the `flatbuffers` project. This project is the api / lib project and contains the test code structure + an example of code generation for testing. This avoid to commit generated code. Pre-configure JUnit for test driven development and make this project OSGi compliant.
  * the `jmh` project. This project aims to provide a placeholder for micro-benchmarking. JMH is a 'de facto' standard for micro benchmarking you can find more details here: http://openjdk.java.net/projects/code-tools/jmh/

For now I didn't move the JavaTest class but it could be a next step with a migration to the JUnit framework.
The only impacts are the move of the class and the project structure => no code change.

Revert "Create a maven like project structure for java development. Make it OSGi compliant. Generate the flatbuffers code for testing (example)."

This reverts commit 9875b0e.

Move maven `pom.xml` from the java folder to the root folder.

This avoid to put the pom.xml file into the source directory. Normally the pom file is in a parent (/parent) folder and it is not mixed with the java source code.
An other thing is: this will make import of the project more easy from a IDE.

The side effect is that the target folder where maven build artifacts will move from the <flatbuffers>/java/target to <flatbuffers>/target therefore the gitignore file has been updated in consequences.

Added OSGi header generation for maven project.
This allow jar generated with maven to be used in OSGi environment.

Fix typo

"your platform can't handling..." => "your platform can't handle"

Fix typo

Related to google#3904 (comment)

fixed ArrayOutOfBoundsException in java example

VS2010 fixes

Xcode fixes

Implement mutators for Go

Added missing Go generated files.

Change-Id: I9d738e84ab2e01ec117c825ade44cc865cf5f1c2

Fixed unused parameter warning.

Change-Id: I7a2576c6f366b89ef3e1f83941f90294ca7a07fd

This is a minimal amount of #ifdef's to make stlport work.

Minimal, in the sense that this will only allow flatbuffers.h +
generated code to work. Everything else (tests, parsing, reflection
etc.) may still not compile with stlport.

Functionality has been reduced, some utility functions are not
available.

Tested: on Linux (no stlport), Android (stlport).

Change-Id: I3f8b6a88258c07d78964dd455fb9f99f65266301

Added way to test two schemas for safe evolution.

Change-Id: I1dfc867e6df5932ab61dad431eb3cb02f15d04df
Tested: on Linux.
Bug: 30202327

Fixed VS2010 build

Fixed conversion warning in generated code.

Emit GetRootAs methods for all types in Go and Python

Add tests for GetRootAs* in Go and Python

Fixed missing \ in CMakeLists.txt that broke the build

js:add @namespace annotation to namespaces

Adding missing generated code from recent commits.

Also updated generated_code.sh to prevent this from happening in
the future.

Change-Id: Ib282e9b6c762a79d4b4e09bee06b14781cd2a4c1

Fixed reflection.h not modifying certain table configurations.

It would write 64bits offsets instead of 32bit ones, and update
the vtable pointer before the fields were processed.

Change-Id: I0c0fa942bbd3b42839294f5653ba8fa048612624
Tested: on Linux.

Pulled out EndianSwap into its own function.

Change-Id: I4a587102db8c435b739c92b6c464c94c4ea1fe42

Fix Mac build.

The new line in the set(CMAKE_CXX_FLAGS...) line was getting insert
verbatim into the Makefile. Makefiles don't like new lines in their
assignment operators.

The newline escape fix works for CMake 3.0 and above, but since
we support 2.x, we need to use the legacy solution, which is to
split into two separate statements.

Tested: cmake -G"Unix Makefiles" works now on Mac.
Change-Id: I6f4655981b85087c4760c3d26ed0c97c4469ba93

Add EnumNames to Go code

Tested: on Darwin

Removed pre-made VS2010 and XCode projects.

CMake is now required on all platforms.

Change-Id: Iad81d9244a05ed70ce8b8860d6b729a873f137c1
Tested: on Windows and OS X.

Fix docs for object API usage

Added AppVeyor CI.

Change-Id: I01cf630026e25382b585785b471bad21153338f6

Need unique_ptr.get not pointee.get

Use fully qualified names of structs in UnPack

feat(mutable-js): The mutable Scalar generation.

This is just the initial commit to start the conversation on adding mutation to javascript.

chore(generate-code): Generate the JS code after mutation has been added.

feat(test): Added mutation testing for scalar values.

This is a port of the tests found in test.cpp

Added backwards compatible --no-union-value-namespacing

Change-Id: Ia78dd3b0f213e9ffa49dcec699dcbb21fe6517da
Tested: on Linux.

Validate UTF-8 by default when parsing IDL. Support Unicode values > U+FFFF in parse
@promethe42

This comment has been minimized.

Copy link

commented Aug 10, 2016

I was trying to send/receive FlatBuffer streams on a socket. Looks like a major feature to me...
Any hope to have this in an actual release anytime soon ?

@gwvo

This comment has been minimized.

Copy link
Contributor

commented Aug 10, 2016

So #3905 was merged, and allows the verifier to compute the size of a FlatBuffer in a larger buffer.

Length-prefixed functionality has not been added yet, and I can't promise when I'll get to it. Meanwhile it is not that hard to do yourself.

@promethe42

This comment has been minimized.

Copy link

commented Aug 10, 2016

So #3905 was merged, and allows the verifier to compute the size of a FlatBuffer in a larger buffer.

I've built master and it should work.
But Verifier::GetComputedSize always returns 0...
Could you provide a sample to show how it works?

@gwvo

This comment has been minimized.

Copy link
Contributor

commented Aug 10, 2016

You'll need to #define FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE before you include flatbuffers.h or the generated code.

@bog-dan-ro

This comment has been minimized.

Copy link
Contributor Author

commented Aug 11, 2016

@promethe42 if Verifier::GetComputedSize returns 0, it means you need to read more data into the buffer.
This is one way to use it:

std::string buff;
while(canReadMoreData) {
 buff.append(moreDataFromSockOrFromDisk);
 flatbuffers::Verifier verifier(buff.c_str(), buff.size());
 if (!VerifyMonsterBuffer(verifier)) // replace VerifyMonsterBuffer with your VerifyXXXXXBuffer function
   continue;
 // Now is safe to create and use the monster from buffer 
 auto monster = GetMonster(buff.c_str());
 // do something with monster

 // Drop the current message and continue reading the next one
 auto msgSize = verifier.GetComputedSize();
 assert(msgSize); // because VerifyMonsterBuffer succeed, msgSize is always > 0
 buff = buff.substr(msgSize);
}
@promethe42

This comment has been minimized.

Copy link

commented Aug 11, 2016

@bog-dan-ro thank you for the sample! So the important part is to call GetComputedSize() after the call to GetXXXXX?

@bog-dan-ro

This comment has been minimized.

Copy link
Contributor Author

commented Aug 11, 2016

@promethe42 , nope the important part is to call it after VerifyMonsterBuffer which is the function that triggers the computation of the size.

@promethe42

This comment has been minimized.

Copy link

commented Aug 11, 2016

@bog-dan-ro OK thanks it works!

I've successfully used FlatBuffers to create a networking protocol between Blender (python) and Minko applications (C++11, https://github.com/aerys/minko). That's pretty cool! Here are some (old) videos of how it's used to do live edition/configuration of a 3D app built/scripted in Blender and developper with the Minko SDK:

http://blogs.aerys.in/jeanmarc-leroux/wp-content/uploads/2015/08/23/Desktop-08.23.2015-18.34.25.07.mp4
http://blogs.aerys.in/jeanmarc-leroux/wp-content/uploads/2015/08/23/Desktop-08.23.2015-18.22.41.06.mp4
http://blogs.aerys.in/jeanmarc-leroux/wp-content/uploads/2015/08/23/Desktop-08.23.2015-14.46.37.04.mp4

Thanks for all the great work/help!

@gwvo

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2016

@promethe42 : looks nice!

@gwvo

This comment has been minimized.

Copy link
Contributor

commented Oct 12, 2016

btw, added some functionality to make size pre-fixing part of FlatBuffers: 486c048

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.