Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Pass -march=x86-64 to build JNI library #164

Merged

Conversation

jmazanec15
Copy link
Member

Issue #, if available:
#163

Description of changes:
This PR changes the way in which we build the JNI library artifacts we release with Opendistro. Previously, we compiled the NMSLIB portion of the library with -march=native. The problem with this is that the machine the Github runner used, had optimized instruction sets that are not present on other machines a user might run the plugin on. This would then lead to an unexpected crash.

This PR changes -march=native to the more generic -march=x86-64 compile option, providing more general compatibility for our library. However, this may lead some users to get worse performance than they otherwise expected, because the distributed binary will no longer include optimized instructions. For users to avoid this, they should build the library from source in an environment that mirrors their production environment. This PR adds this suggestion in the README as well.

In order to test, I built the RPM on an Amazon Linux 2 machine on an m5.xlarge EC2 instance and then installed it on an m4.xlarge instance. I was able to confirm that the steps from #163 that led to the crash earlier, did not lead to a crash with this method.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jmazanec15 jmazanec15 requested a review from vamshin July 14, 2020 21:40
@jmazanec15 jmazanec15 changed the title Action compile fix FIX: Pass -march=x86-64 to build JNI library Jul 14, 2020
Copy link
Member

@vamshin vamshin left a comment

Choose a reason for hiding this comment

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

LGTM!

@jmazanec15 jmazanec15 merged commit 68950f5 into opendistro-for-elasticsearch:master Jul 15, 2020
chenqi0805 added a commit to chenqi0805/k-NN that referenced this pull request Jul 15, 2020
…asure

* master:
  FIX: Pass -march=x86-64 to build JNI library (opendistro-for-elasticsearch#164)
  FIX: Added resetState for uTs so state does not spill over (opendistro-for-elasticsearch#159)
@jmazanec15 jmazanec15 changed the title FIX: Pass -march=x86-64 to build JNI library Pass -march=x86-64 to build JNI library Jul 16, 2020
@jmazanec15 jmazanec15 added the Infrastructure Change releated to distribution of plugin label Jul 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Infrastructure Change releated to distribution of plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants