richardstartin commented on a change in pull request #7844:
URL: https://github.com/apache/pinot/pull/7844#discussion_r759747251



##########
File path: 
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/PartitionUpsertMetadataManager.java
##########
@@ -186,27 +182,32 @@ public GenericRow updateRecord(IndexSegment segment, 
RecordInfo recordInfo, Gene
       }
     }
 
+    ThreadSafeMutableRoaringBitmap validDocIds = segment.getValidDocIds();
+    assert validDocIds != null;
     _result = record;
-    _primaryKeyToRecordLocationMap
-        .compute(hashPrimaryKey(recordInfo._primaryKey, _hashFunction), 
(primaryKey, currentRecordLocation) -> {
+    
_primaryKeyToRecordLocationMap.compute(hashPrimaryKey(recordInfo._primaryKey, 
_hashFunction),
+        (primaryKey, currentRecordLocation) -> {
           if (currentRecordLocation != null) {
             // Existing primary key
 
-            // Update the record location when the new comparison value is 
greater than or equal to the current value
-            // . Update
-            // the record location when there is a tie to keep the newer 
record.
+            // Update the record location when the new comparison value is 
greater than or equal to the current value.
+            // Update the record location when there is a tie to keep the 
newer record.
             if 
(recordInfo._comparisonValue.compareTo(currentRecordLocation.getComparisonValue())
 >= 0) {
               IndexSegment currentSegment = currentRecordLocation.getSegment();
+              int currentDocId = currentRecordLocation.getDocId();
               if (_partialUpsertHandler != null) {
                 // Partial upsert
                 _reuse.clear();
-                GenericRow previousRecord = 
currentSegment.getRecord(currentRecordLocation.getDocId(), _reuse);
+                GenericRow previousRecord = 
currentSegment.getRecord(currentDocId, _reuse);
                 _result = _partialUpsertHandler.merge(previousRecord, record);
               }
-              assert currentSegment.getValidDocIds() != null;

Review comment:
       Also, don’t be concerned about adding null checks, they aren’t compiled 
the way you see them - they get speculatively optimised away based on profiling 
data collected in lower tiers. So this is not more efficient than a null check 
which never fails, it’s exactly as efficient but with more bytecode weight. 
This is a good read on the topic: 
https://shipilev.net/jvm/anatomy-quarks/25-implicit-null-checks/




-- 
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: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to