Fixed bug in checking for unused ordinals

stevenewald · Aug 12, 2023 · 1d09ae8 · 1d09ae8
1 parent 467ff63
commit 1d09ae8
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 12 deletions.
diff --git a/hollow/src/main/java/com/netflix/hollow/tools/history/keyindex/HollowOrdinalMapper.java b/hollow/src/main/java/com/netflix/hollow/tools/history/keyindex/HollowOrdinalMapper.java
@@ -90,10 +90,13 @@ private boolean recordsAreEqual(HollowObjectTypeReadState typeState, int keyOrdi
 
             Object newFieldValue = readValueInState(typeState, keyOrdinal, fieldIdx);
             int existingFieldOrdinalValue = indexFieldOrdinalMapping.get(index)[fieldIdx];
-            if(memoizedPool.writtenInCycle(existingFieldOrdinalValue))
+
+            //Assuming two records in the same cycle cannot be equal
+            if(memoizedPool.ordinalInCurrentCycle(existingFieldOrdinalValue)) {
                 return false;
+            }
             Object existingFieldObjectValue = memoizedPool.getObject(existingFieldOrdinalValue, keyFieldTypes[fieldIdx]);
-            if(!newFieldValue.equals(existingFieldObjectValue)) {
+            if (!newFieldValue.equals(existingFieldObjectValue)) {
                 return false;
             }
         }

diff --git a/hollow/src/main/java/com/netflix/hollow/tools/util/ObjectInternPool.java b/hollow/src/main/java/com/netflix/hollow/tools/util/ObjectInternPool.java
@@ -6,6 +6,7 @@
 import com.netflix.hollow.core.memory.encoding.VarInt;
 import com.netflix.hollow.core.schema.HollowObjectSchema.FieldType;
 import java.util.HashSet;
+import java.util.Optional;
 
 
 // This class memoizes types by returning references to existing objects, or storing
@@ -20,10 +21,6 @@ public ObjectInternPool() {
         this.ordinalsInCycle = new HashSet<>();
     }
 
-    public boolean writtenInCycle(int ordinal) {
-        return ordinalsInCycle.contains(ordinal);
-    }
-
     public void prepareForRead() {
         if(!isReadyToRead) {
             ordinalMap.prepareForWrite();
@@ -32,14 +29,12 @@ public void prepareForRead() {
         isReadyToRead = true;
     }
 
-    //WARNING: assumes already ready to read
-    public Object getObject(int ordinal, FieldType type) {
-        prepareForRead();
+    public boolean ordinalInCurrentCycle(int ordinal) {
+        return ordinalsInCycle.contains(ordinal);
+    }
 
+    public Object getObject(int ordinal, FieldType type) {
         long pointer = ordinalMap.getPointerForData(ordinal);
-        if (pointer==-1L) {
-            return null;
-        }
 
         switch (type) {
             case BOOLEAN:

diff --git a/hollow/src/test/java/com/netflix/hollow/tools/util/ObjectInternPoolTest.java b/hollow/src/test/java/com/netflix/hollow/tools/util/ObjectInternPoolTest.java
@@ -38,6 +38,7 @@ public void testInt() {
 
         int int1Ordinal = internPool.writeAndGetOrdinal(intObj1);
         int int2Ordinal = internPool.writeAndGetOrdinal(intObj2);
+        internPool.prepareForRead();
 
         int retrievedInt1 = (int) internPool.getObject(int1Ordinal, FieldType.INT);
 
@@ -49,6 +50,7 @@ public void testInt() {
 
         int int3Ordinal = internPool.writeAndGetOrdinal(intObj3);
         int int4Ordinal = internPool.writeAndGetOrdinal(intObj4);
+        internPool.prepareForRead();
 
         int retrievedInt3 = (int) internPool.getObject(int3Ordinal, FieldType.INT);
 
@@ -65,6 +67,7 @@ public void testFloat() {
 
         int float1Ordinal = internPool.writeAndGetOrdinal(floatObj1);
         int float2Ordinal = internPool.writeAndGetOrdinal(floatObj2);
+        internPool.prepareForRead();
 
         float retrievedFloat1 = (float) internPool.getObject(float1Ordinal, FieldType.FLOAT);
 
@@ -73,6 +76,7 @@ public void testFloat() {
 
         int float3Ordinal = internPool.writeAndGetOrdinal(floatObj3);
         int float4Ordinal = internPool.writeAndGetOrdinal(floatObj4);
+        internPool.prepareForRead();
 
         float retrievedFloat2 = (float) internPool.getObject(float3Ordinal, FieldType.FLOAT);
 
@@ -89,6 +93,7 @@ public void testLong() {
 
         int long1Ordinal = internPool.writeAndGetOrdinal(longObj1);
         int long2Ordinal = internPool.writeAndGetOrdinal(longObj2);
+        internPool.prepareForRead();
 
         long retrievedLong1 = (Long) internPool.getObject(long1Ordinal, FieldType.LONG);
 
@@ -97,6 +102,7 @@ public void testLong() {
 
         int long3Ordinal = internPool.writeAndGetOrdinal(longObj3);
         int long4Ordinal = internPool.writeAndGetOrdinal(longObj4);
+        internPool.prepareForRead();
 
         long retrievedLong2 = (Long) internPool.getObject(long3Ordinal, FieldType.LONG);
 
@@ -113,6 +119,7 @@ public void testDouble() {
 
         int double1Ordinal = internPool.writeAndGetOrdinal(doubleObj1);
         int double2Ordinal = internPool.writeAndGetOrdinal(doubleObj2);
+        internPool.prepareForRead();
 
         double retrievedDouble1 = (double) internPool.getObject(double1Ordinal, FieldType.DOUBLE);
 
@@ -121,6 +128,7 @@ public void testDouble() {
 
         int double3Ordinal = internPool.writeAndGetOrdinal(doubleObj3);
         int double4Ordinal = internPool.writeAndGetOrdinal(doubleObj4);
+        internPool.prepareForRead();
 
         double retrievedDouble2 = (double) internPool.getObject(double3Ordinal, FieldType.DOUBLE);
 
@@ -137,6 +145,7 @@ public void testString() {
 
         int string1Ordinal = internPool.writeAndGetOrdinal(stringObj1);
         int string2Ordinal = internPool.writeAndGetOrdinal(stringObj2);
+        internPool.prepareForRead();
 
         String retrievedString1 = (String) internPool.getObject(string1Ordinal, FieldType.STRING);
 
@@ -145,6 +154,7 @@ public void testString() {
 
         int string3Ordinal = internPool.writeAndGetOrdinal(stringObj3);
         int string4Ordinal = internPool.writeAndGetOrdinal(stringObj4);
+        internPool.prepareForRead();
 
         String retrievedString2 = (String) internPool.getObject(string3Ordinal, FieldType.STRING);
 
@@ -161,6 +171,7 @@ public void testBool() {
 
         int bool1Ordinal = internPool.writeAndGetOrdinal(boolObj1);
         int bool2Ordinal = internPool.writeAndGetOrdinal(boolObj2);
+        internPool.prepareForRead();
 
         boolean retrievedBool1 = (boolean) internPool.getObject(bool1Ordinal, FieldType.BOOLEAN);
 
@@ -169,6 +180,7 @@ public void testBool() {
 
         int bool3Ordinal = internPool.writeAndGetOrdinal(boolObj3);
         int bool4Ordinal = internPool.writeAndGetOrdinal(boolObj4);
+        internPool.prepareForRead();
 
         boolean retrievedBool2 = (boolean) internPool.getObject(bool3Ordinal, FieldType.BOOLEAN);