-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
Enable madvise by default for all builds #110159
Enable madvise by default for all builds #110159
Conversation
Pinging @elastic/es-search (Team:Search) |
@elasticmachine update branch |
distribution/src/config/jvm.options
Outdated
@@ -58,6 +58,9 @@ | |||
# result in less optimal vector performance | |||
20-:--add-modules=jdk.incubator.vector | |||
|
|||
# Enable random access flags via madvise | |||
-Des.madv_random_feature_flag_enabled=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think typically when we want to enable something by default that was previously behind a feature flag, we simply remove the feature flag. In either case, I don't think we'd want this option in jvm.options
but rather in SystemJvmOptions
. @rjernst might provide more insight here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right, we should remove the feature flag to "enable" it permanently. If the intent is for users to be able to disable this, then it should be moved from a feature flag to a setting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
roger! I will just remove the flag :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the complete removal of the feature flag. LGTM
@elasticmachine update branch |
This feature flag has been enabled by default for snapshot builds for some time, no significant bumps or changes in rally. This commit will enable it by default, even in release builds.
This feature flag has been enabled by default for snapshot builds for some time, no significant bumps or changes in rally. This commit will enable it by default, even in release builds.
This feature flag has been enabled by default for snapshot builds for some time, no significant bumps or changes in rally.
This commit will enable it by default, even in release builds.