Compute declared versions in a static block

[scottsom]

This method is called often enough that the reflection and sort
can be seen while profiling. The impact is quite small but
it is an easy enough fix and I see no reason why this needs to be
computed every invoke.

[elasticmachine]

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

[elasticmachine]

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

[jasontedor]

Thanks @scottsom, I left some feedback.

[jasontedor]

Please make this lazy, and make this an unmodifiable list. Also, I think we can drop this method and only access the list?

[scottsom]

I think the method needs to stay since it can be called with a different version class:
https://github.com/elastic/elasticsearch/blob/master/test/framework/src/main/java/org/elasticsearch/test/VersionUtils.java#L53

[jasontedor]

Yet you can expose the list to that class too?

[scottsom]

The input in the VersionUtils method is an arbitrary Class<?> rather than Version.class, so the values in the list will change based on the input.

[jasontedor]

I see, thanks for investigating.

[jasontedor]

I left a suggestion, let me know what you think?

[jasontedor]

I was thinking something like:

diff --git a/server/src/main/java/org/elasticsearch/Version.java b/server/src/main/java/org/elasticsearch/Version.java
index 1e1993e6d6..c0181d4f1e 100644
--- a/server/src/main/java/org/elasticsearch/Version.java
+++ b/server/src/main/java/org/elasticsearch/Version.java
@@ -372,7 +372,9 @@ public class Version implements Comparable<Version> {
     public final byte build;
     public final org.apache.lucene.util.Version luceneVersion;
 
-    private List<Version> declaredVersionsCache;
+    private static final class DeclaredVersionsHolder {
+        static final List<Version> declaredVersions = Collections.unmodifiableList(getDeclaredVersions(Version.class));
+    }
 
     Version(int id, org.apache.lucene.util.Version luceneVersion) {
         this.id = id;
@@ -414,17 +416,10 @@ public class Version implements Comparable<Version> {
     public Version minimumCompatibilityVersion() {
         if (major >= 6) {
             // all major versions from 6 onwards are compatible with last minor series of the previous major
-            List<Version> declaredVersions = declaredVersionsCache;
-
-            if (declaredVersions == null) {
-                declaredVersions = Collections.unmodifiableList(getDeclaredVersions(getClass()));
-                declaredVersionsCache = declaredVersions;
-            }
-
             Version bwcVersion = null;
 
-            for (int i = declaredVersions.size() - 1; i >= 0; i--) {
-                final Version candidateVersion = declaredVersions.get(i);
+            for (int i = DeclaredVersionsHolder.declaredVersions.size() - 1; i >= 0; i--) {
+                final Version candidateVersion = DeclaredVersionsHolder.declaredVersions.get(i);
                 if (candidateVersion.major == major - 1 && candidateVersion.isRelease() && after(candidateVersion)) {
                     if (bwcVersion != null && candidateVersion.minor < bwcVersion.minor) {
                         break;

[scottsom]

Looks good to me, I was copying the String#hashCode style. I'll make that change.

[jasontedor]

test this please

[jasontedor]

LGTM.

[jasontedor]

Thanks @scottsom.