epotyom commented on code in PR #15627:
URL: https://github.com/apache/lucene/pull/15627#discussion_r2742153654


##########
lucene/core/src/java/org/apache/lucene/index/TermStates.java:
##########
@@ -193,28 +193,25 @@ public IOSupplier<TermState> get(LeafReaderContext ctx) 
throws IOException {
         this.states[ctx.ord] = EMPTY_TERMSTATE;
         return null;
       }
-      return () -> {
-        if (this.states[ctx.ord] == null) {
-          TermState state = null;
-          if (termExistsSupplier.get()) {
-            state = termsEnum.termState();
-            this.states[ctx.ord] = state;
-          } else {
-            this.states[ctx.ord] = EMPTY_TERMSTATE;
-          }
-        }
-        TermState state = this.states[ctx.ord];
-        if (state == EMPTY_TERMSTATE) {
-          return null;
-        }
-        return state;
-      };
-    }
-    TermState state = this.states[ctx.ord];
-    if (state == EMPTY_TERMSTATE) {
-      return null;
+      IOSupplier<TermState> stateSupplier =
+          () -> {
+            if (this.states[ctx.ord] == null) {
+              if (termExistsSupplier.get()) {
+                this.states[ctx.ord] = termsEnum.termState();
+              } else {
+                this.states[ctx.ord] = EMPTY_TERMSTATE;
+              }
+            }
+            return this.states[ctx.ord] == EMPTY_TERMSTATE ? null : 
this.states[ctx.ord];
+          };
+      if (termExistsSupplier.doDefer()) {
+        return stateSupplier;
+      } else {
+        stateSupplier.get();
+        return this.states[ctx.ord] == EMPTY_TERMSTATE ? null : () -> 
this.states[ctx.ord];

Review Comment:
   Same as above - we night want to use stateSupplier.get() results instead



##########
lucene/core/src/java/org/apache/lucene/index/TermStates.java:
##########
@@ -193,28 +193,25 @@ public IOSupplier<TermState> get(LeafReaderContext ctx) 
throws IOException {
         this.states[ctx.ord] = EMPTY_TERMSTATE;
         return null;
       }
-      return () -> {
-        if (this.states[ctx.ord] == null) {
-          TermState state = null;
-          if (termExistsSupplier.get()) {
-            state = termsEnum.termState();
-            this.states[ctx.ord] = state;
-          } else {
-            this.states[ctx.ord] = EMPTY_TERMSTATE;
-          }
-        }
-        TermState state = this.states[ctx.ord];
-        if (state == EMPTY_TERMSTATE) {
-          return null;
-        }
-        return state;
-      };
-    }
-    TermState state = this.states[ctx.ord];
-    if (state == EMPTY_TERMSTATE) {
-      return null;
+      IOSupplier<TermState> stateSupplier =
+          () -> {
+            if (this.states[ctx.ord] == null) {
+              if (termExistsSupplier.get()) {
+                this.states[ctx.ord] = termsEnum.termState();
+              } else {
+                this.states[ctx.ord] = EMPTY_TERMSTATE;
+              }
+            }
+            return this.states[ctx.ord] == EMPTY_TERMSTATE ? null : 
this.states[ctx.ord];

Review Comment:
   I think before we kept `this.states[ctx.ord]` in a variable to guarantee 
single array read? Not sure if hotspot always optimizes it.



##########
lucene/core/src/java/org/apache/lucene/index/TermStates.java:
##########
@@ -193,28 +193,25 @@ public IOSupplier<TermState> get(LeafReaderContext ctx) 
throws IOException {
         this.states[ctx.ord] = EMPTY_TERMSTATE;
         return null;
       }
-      return () -> {
-        if (this.states[ctx.ord] == null) {
-          TermState state = null;
-          if (termExistsSupplier.get()) {
-            state = termsEnum.termState();
-            this.states[ctx.ord] = state;
-          } else {
-            this.states[ctx.ord] = EMPTY_TERMSTATE;
-          }
-        }
-        TermState state = this.states[ctx.ord];
-        if (state == EMPTY_TERMSTATE) {
-          return null;
-        }
-        return state;
-      };
-    }
-    TermState state = this.states[ctx.ord];
-    if (state == EMPTY_TERMSTATE) {
-      return null;
+      IOSupplier<TermState> stateSupplier =
+          () -> {
+            if (this.states[ctx.ord] == null) {
+              if (termExistsSupplier.get()) {
+                this.states[ctx.ord] = termsEnum.termState();
+              } else {
+                this.states[ctx.ord] = EMPTY_TERMSTATE;
+              }
+            }
+            return this.states[ctx.ord] == EMPTY_TERMSTATE ? null : 
this.states[ctx.ord];
+          };
+      if (termExistsSupplier.doDefer()) {
+        return stateSupplier;
+      } else {
+        stateSupplier.get();
+        return this.states[ctx.ord] == EMPTY_TERMSTATE ? null : () -> 
this.states[ctx.ord];
+      }
     }
-    return () -> state;
+    return this.states[ctx.ord] == EMPTY_TERMSTATE ? null : () -> 
this.states[ctx.ord];

Review Comment:
   Same as above



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to