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

RocksDB changes to support unity build: #168

Closed
wants to merge 1 commit into from
Closed

RocksDB changes to support unity build: #168

wants to merge 1 commit into from

Conversation

vinniefalco
Copy link

This set of changes allows RocksDB to be compiled as a "unity build" (http://buffered.io/posts/the-magic-of-unity-builds/). I realize that this is an odd use-case: we inline the entire rocksdb repository within our own respotory using git subtree (http://blogs.atlassian.com/2013/05/alternatives-to-git-submodule-git-subtree/). But the RocksDB sources only needed very minor cosmetic changes. Mostly eliminating duplicate symbols in different translation units. And also, moving some identifiers out of anonymous namespaces to prevent compilation warnings when those symbols appear as data members of classes in non-anonymous namespaces.

Here's the source file used to compile RocksDB as a unity build:
https://github.com/ripple/rippled/blob/develop/src/ripple/unity/rocksdb.cpp

* Remove extra definition of TotalFileSize
* Remove extra definition of ClipToRange
* Move EncodedFileMetaData out from anonymous namespace
* Move version_set Saver to its own namespace
* Move some symbols into a named namespace
* Move symbols out of anonymous namespace (prevents warning)
* Make BloomHash inline
@dhruba
Copy link
Contributor

dhruba commented Jun 5, 2014

Once this patch gets committed, how do you plan to ensure that new patches do not break the unity build? Do you plan to setup some open-source build process that builds rocksdb nightly and posts the results to this group?

@vinniefalco
Copy link
Author

Well every time we update our RocksDB subtree I would fix the errors and submit a new pull request. This might not happen with every commit in the upstream though. If you wanted that, then we should consider adding a target to the RocksDB build that compiles the unity .cpp (which you can add to the RocksDB repository).Of course the unity style build is not what the majority of programmers are used to seeing. But if it's any consolation, it compiles really fast. This is a godsend when building from scratch on Travis.

@siying
Copy link
Contributor

siying commented Jul 3, 2014

It looks good to me. Can you rebase?

@vinniefalco
Copy link
Author

Yes. I had to reshuffle things a bit. Going forward, I believe the best way to ensure support for this style of build is to add a target to the RocksDB build system that compiles a unity .cpp file and checks for errors. It's fairly easy to model one after this:
https://github.com/ripple/rippled/blob/develop/src/ripple/unity/rocksdb.cpp
What do you think?

@vinniefalco
Copy link
Author

Any word on this? We found some defects in usage of naked pointers in the API (specifically the bloom filter) and we'd like to get some convergence on the diverging branches

@siying
Copy link
Contributor

siying commented Aug 1, 2014

@vinniefalco I'm fine with it. You are welcome to improve our makefile file to add a target for it.

@siying
Copy link
Contributor

siying commented Aug 1, 2014

I can pull this patch once the conflicts are resolved.

@vinniefalco
Copy link
Author

I'm going to close this for now, the merge conflicts are somewhat non-trivial. I'll get to work on a new pull request and then re-submit. Thanks

@vinniefalco vinniefalco closed this Aug 4, 2014
Little-Wallace pushed a commit to Little-Wallace/rocksdb that referenced this pull request Apr 30, 2020
…acebook#167) (facebook#168)

Summary:
Fix NewRandomRWFile and ReuseWritableFile misuse of `GetFile()` and `NewFile()`. See inline comments.

Test Plan:
manual test with tikv

Signed-off-by: Yi Wu <[email protected]>
Nazgolze pushed a commit to Nazgolze/rocksdb-1 that referenced this pull request Sep 21, 2021
luky116 pushed a commit to luky116/rocksdb that referenced this pull request Jul 10, 2024
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

Successfully merging this pull request may close these issues.

3 participants