Skip to content
This repository has been archived by the owner on Feb 26, 2020. It is now read-only.

Support old CPUs #22

Merged
merged 3 commits into from
Oct 9, 2018
Merged

Support old CPUs #22

merged 3 commits into from
Oct 9, 2018

Conversation

ordian
Copy link
Member

@ordian ordian commented Oct 8, 2018

I could modify CMakeLists.txt to something like -march=core2 and /arch:AVX on Windows, but not sure it is the best way, since it would probably create conflicts when updating RocksDB and there are some checks like this one that needs to be removed as well. Also I haven't tested this (don't have a new CPU atm) and don't know what the performance impact is.

Alternatively, we can say that the binaries support only new CPUs and one needs to build from source on an old CPU if needed, but that doesn't seem like a good approach to me, especially in case of Windows.

related to openethereum/parity-ethereum#9684.

Copy link

@andresilva andresilva left a comment

Choose a reason for hiding this comment

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

I have mixed feelings about this since this will disable more than SSE4.2 (thread locals, fallocate, sync_file_range and adaptive mutexes). This probably doesn't impact performance that much (we were already building with all of these features disabled until recently), but it's one of the main reasons I wanted to migrate the build to CMake. Unfortunately I already had to do make some changes to CMakeLists.txt to make it build properly for ARM, all commit messages that start with rocksdb: basically change something in the rocksdb subtree, so yeah we might have to deal with some conflicts when we pull again from rocksdb (I also had to cherry-pick some commits to fix build issues IIRC). Still, this is probably the easiest/fastest way to fix the issue right now and publish a parity-ethereum binary that doesn't cause any compatibility issues. I guess we can create an issue to remove the PORTABLE flag in the future and make more fine-grained adjustments to CMakeLists.txt.

}
// Added to support old CPUs
// see https://github.com/paritytech/parity-ethereum/issues/9684
cfg.define("PORTABLE", "ON");
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we could use an env variable to allow compiling with PORTABLE=OFF, so that we distribute portable binaries, but allow compiling from source with all the features for ultimate performance?

@andresilva
Copy link

Good idea, I was going to suggest using a cargo feature but it's probably easier to use an environment variable (since with a feature we'd need to add it to kvdb-rocksdb as well so that it's reachable from parity-ethereum).

@ordian ordian merged commit 578c41e into master Oct 9, 2018
@ordian ordian deleted the support-old-cpus branch October 9, 2018 14:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants