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

update protobuf lib to the latest release 3.6 #6

Closed
mikejiang opened this issue Nov 20, 2018 · 5 comments
Closed

update protobuf lib to the latest release 3.6 #6

mikejiang opened this issue Nov 20, 2018 · 5 comments

Comments

@mikejiang
Copy link
Member

The current bundled pb is 2.6, which is 4 year old. We will have to test the backward compatibility (for both code and data archive) before making such upgrade.

@mikejiang
Copy link
Member Author

In file included from /Users/Mike/Library/R/3.6/library/RProtoBufLib/include/GatingSet.pb.h:22:
In file included from /Users/Mike/Library/R/3.6/library/RProtoBufLib/include/google/protobuf/generated_message_util.h:44:
In file included from /Users/Mike/Library/R/3.6/library/RProtoBufLib/include/google/protobuf/stubs/once.h:81:
In file included from /Users/Mike/Library/R/3.6/library/RProtoBufLib/include/google/protobuf/stubs/atomicops.h:179:
/Users/Mike/Library/R/3.6/library/RProtoBufLib/include/google/protobuf/stubs/atomicops_internals_macosx.h:164:10: warning: 'OSAtomicAdd64Barrier' is deprecated: first deprecated in macOS 10.12 -In file in

The compiling warnings are flooding the flowWorkspace package building process, which could make it difficult to troubleshoot the real compiling errors.

@mikejiang
Copy link
Member Author

pb is upgraded to 3.7 now in cytoset branch (with 6b96842).
Tthe proto message file is kept unchanged because switching to proto3 syntax will cause some breaking changes to the cytolib code due to the changing behavior of optional fields, i.e. the missing parent reference now returns a default 0 value instead of null, which will be ambiguous in our context since we interpret 0 as root node.

Preliminary tests show that it can successfully load the gs archived by pb2.6.
Right now the bundled pb includes everything and we can trim it down later if the speed of RProtobufLib package building ever becomes an issue.
Also the bundled windows dll can be updated later as well,but it is not urgent either since pb2.6 and pb3.7 play nicely with each other without any issues.

@mikejiang
Copy link
Member Author

We can potentially further optimize the pb code footprint by only using protobuf lite, which will downsize the protocol buffers runtime significantly (libprotobuf.so 31M -- > libprotobuf-lite.so 4.8M)as well as reduce the interface overheads.

mikejiang pushed a commit that referenced this issue Mar 19, 2019
mikejiang pushed a commit that referenced this issue Mar 19, 2019
@mikejiang
Copy link
Member Author

tests show protobuf-lite doesn't improve load_gs much. But since it requires more lightweight runtime, I will still use it.

@mikejiang
Copy link
Member Author

We will hold off merging these to trunk until windows static lib for protobuf-lite3.7 is built and tested for cs branch.

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

No branches or pull requests

1 participant