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

Build rocksdb as a portable build #42

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

silviucpp
Copy link

No description provided.

@silviucpp
Copy link
Author

Most of the time you don't have the guarantee that build machines supports same SIMD instructions like the machines where the app will run.

In case the build is not portable and on build machine you have some instructions that are not supported on the servers where the app will run you will have crashes.

PORTABLE=1 fix this.

@mocchira mocchira self-requested a review June 24, 2019 13:37
@Licenser
Copy link

I ran into similar issues before and when I did my research back then I found that the dependency is AVX/SSE4.2 both were introduced around 2011 in both AMD and Intel. I think however that was with leveldb. Not sure if they are the same with rocksdb but given it is a fork of leveldb it might very well be.

I'm not sure if penalizing the performance on modern systems to allow rocksdb to work/build on 8+-year-old machines without re-compilation is the right step.

@silviucpp
Copy link
Author

I'm not sure if only hardware older than 8+ will not have this features. In my case I'm trying to run a build compiled on a physical machine in a VM which runs on a newer hardware than the physical.

And is not working. The app crashes. Looking to official rocksdb build scripts for docker they are using PORTABLE=1

@Licenser
Copy link

That’s quite different from what I saw and clearly changes the trade offs

@silviucpp
Copy link
Author

Usually a better way is to detect at run-time what features the CPU supports and enable/disable them. This is what lot of libs are doing like libvpx, libyuv etc. But unfortunately rocksdb was not designed in this way

@mocchira
Copy link
Member

WIP

@mocchira
Copy link
Member

@silviucpp Confirmed that it works on several environments however I think the default value for PORTABLE should be kept (=0) for the (performance) backward compatibility for existing users.

so how about the below approach?

### Comment out the below line when your app crashes on the machine where your app will run
### For more information, please visit https://github.com/facebook/rocksdb/blob/master/INSTALL.md#compilation explaining when you should use "PORTABLE=1"

#PORTABLE=1

@silviucpp
Copy link
Author

I'm thinking if there is any better way to pass to rebar3 before compiling an env variable.. Otherwise users that needs to use PORTABLE=1 needs to fork the project, commit the change and use their own forks.

@mocchira
Copy link
Member

mocchira commented Aug 9, 2019

@silviucpp https://github.com/rebar/rebar/wiki/Dynamic-configuration seems to work as you intend.

  1. Check the environment variable PORTABLE in rebar.config.script
  2. If it exists then replace prehooks with the below one
{pre_hooks, [{'get-deps', "c_src/build_deps.sh get-deps"},
             {compile, "PORTABLE=1 c_src/build_deps.sh"}
            ]}.
  1. set $PORTABLE passed from rebar export PORTABLE=$PORTABLE in c_src/build_deps.sh

I will try in my spare time.

@silviucpp
Copy link
Author

Ohh thanks ! this is nice, I'll give a try.

I think you should put in readme this for every one else that might encounter this problem.

Silviu

@mocchira
Copy link
Member

I think you should put in readme this for every one else that might encounter this problem.

Yes I will later. Thanks for pointing out that.

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