Reviewing files that changed from the base of the PR and between 9adf861 and 0524ba5.

• lib/common/common/src/counter/counter_cell.rs (1 hunks)
• lib/common/common/src/counter/hardware_counter.rs (2 hunks)
• lib/segment/src/index/field_index/full_text_index/mutable_inverted_index.rs (2 hunks)
• lib/segment/src/index/field_index/geo_index/mutable_geo_index.rs (2 hunks)
• lib/segment/src/index/field_index/map_index/mod.rs (1 hunks)
• lib/segment/src/index/field_index/map_index/mutable_map_index.rs (2 hunks)
• lib/segment/src/index/field_index/mmap_point_to_values.rs (2 hunks)
• lib/segment/src/index/field_index/numeric_index/immutable_numeric_index.rs (2 hunks)
• lib/segment/src/index/field_index/numeric_index/mmap_numeric_index.rs (2 hunks)
• lib/segment/src/index/field_index/numeric_index/mutable_numeric_index.rs (1 hunks)

• GitHub Check: test-shard-snapshot-api-s3-minio
• GitHub Check: Basic TLS/HTTPS tests
• GitHub Check: test-snapshot-operations-s3-minio
• GitHub Check: test-low-resources
• GitHub Check: test-consistency
• GitHub Check: test-consensus-compose
• GitHub Check: test (macos-latest)
• GitHub Check: test
• GitHub Check: test (windows-latest)
• GitHub Check: test-consensus
• GitHub Check: test
• GitHub Check: test (ubuntu-latest)
• GitHub Check: test

lib/common/common/src/counter/hardware_counter.rs (3)

213-216: Good addition of write-back counter to tests

The write-back counter is now properly tested in the test_hw_counter_drain method. This ensures that when a write-back counter is used and dropped, its increments are properly accumulated in the original counter.

---

219-219: Correctly updated assertion for cumulative counter value

Good adjustment of the expected value from 1 to 2, accounting for both the direct increment (line 211) and the write-back counter increment (line 215).

---

237-240: Appropriate write-back counter usage in accumulator test

Effective demonstration of using write-back counters alongside direct counter increments. This change properly tests the accumulation of both increment methods to ensure they work together correctly.

lib/segment/src/index/field_index/numeric_index/mmap_numeric_index.rs (2)

6-6: Good addition of required import

Correctly added the import for HwMeasurementIteratorExt which provides the measure_hw_with_cell method used in the updated code.

---

258-260: Improved hardware counter measurement approach

Replaced the previous inspect-based counter incrementing pattern with a more efficient and cleaner measure_hw_with_cell approach. This change improves code readability and maintainability while following the new counter pattern.

lib/segment/src/index/field_index/mmap_point_to_values.rs (2)

287-295: Enhanced hardware counter handling with write-back pattern

Good update to use the write-back counter pattern. The parameter rename from hw_acc to hw_counter improves naming consistency, and using a write-back counter through write_back_counter() follows the new efficient counter pattern.

---

303-303: Correctly updated increment to use write-back counter

Updated the increment call to use the new write-back counter mechanism, ensuring hardware measurements are properly accumulated.

lib/segment/src/index/field_index/numeric_index/immutable_numeric_index.rs (2)

7-7: Added required HwMeasurementIteratorExt import

Correctly imported the trait that provides the measure_hw_with_cell method used in the updated code.

---

225-227: Improved hardware counter measurement in immutable index

Replaced the previous hardware counter incrementing approach with the cleaner measure_hw_with_cell pattern. This change provides a more consistent and efficient way to measure hardware usage across the codebase.

lib/segment/src/index/field_index/geo_index/mutable_geo_index.rs (3)

329-331: Good use of write-back counter pattern.

The implementation correctly obtains a write-back counter from the hardware counter cell, which will accumulate increments locally and write back to the original counter when dropped.

---

337-337: Properly measuring geo hash size with write-back counter.

The code now directly measures each geo hash's size with the write-back counter, which is more efficient than accumulating counts in a local variable.

---

351-351: Efficiently measuring total point offset size.

The implementation correctly calculates the total memory size for all point offsets at once, which is an efficient approach.

lib/segment/src/index/field_index/numeric_index/mutable_numeric_index.rs (2)

265-268: Good use of write-back counter pattern.

The implementation correctly obtains a write-back counter from the hardware counter cell, which accumulates increments locally and writes back to the original counter when dropped.

---

272-273: Correctly measuring key and index size in each iteration.

The code now directly increments the write-back counter with both the key and index sizes in each loop iteration, eliminating the need for a local counter variable.

lib/segment/src/index/field_index/full_text_index/mutable_inverted_index.rs (4)

86-88: Good use of write-back counter pattern.

The implementation correctly obtains a write-back counter from the hardware counter cell, which will accumulate increments locally and write back to the original counter when dropped.

---

94-95: Accurately measuring Document overhead.

The code now uses the write-back counter to track the memory overhead of expanding the point_to_docs vector, with a clear comment explaining that it's only measuring the Document's overhead.

---

105-106: Properly measuring postings expansion size.

The write-back counter correctly measures the size difference when the postings vector needs to be expanded.

---

118-118: Accurately tracking point ID size.

The implementation correctly measures the size of each point ID with the write-back counter.

lib/segment/src/index/field_index/map_index/mod.rs (3)

506-508: Good use of write-back counter pattern.

The implementation correctly obtains a write-back counter from the hardware counter cell, accumulating increments locally and writing back to the original counter when dropped.

---

515-515: Correctly measuring unique value size.

The write-back counter now tracks the size of each unique value added to the index, which is more direct than using a local counter variable.

---

518-518: Properly measuring point ID size.

The implementation correctly measures the size of each point ID with the write-back counter.

lib/common/common/src/counter/counter_cell.rs (4)

66-72: New method provides performance-optimized counting

The write_back_counter method creates a new WritebackCounterCell that accumulates increments and writes back to the original counter only when dropped. This is a good design for optimizing performance in hot-paths and loops.

---

100-105: Drop implementation ensures proper counter updates

The drop implementation correctly writes the accumulated count back to the original cell, which is the core mechanism that makes this optimization work without requiring manual reconciliation.

---

107-117: Simple and efficient WritebackCounterCell implementation

The implementation is clean and focused. The incr_delta method performs a simple addition without any cell access, which is exactly what's needed for the performance optimization.

---

119-134: Test validates core functionality but can be expanded

The test correctly verifies that the counter value is written back on drop. However, based on an earlier review comment, adding an additional assertion after incrementing but before dropping would better demonstrate the behavior.

Consider enhancing the test to include additional assertions and test cases:

 #[test]
 fn test_write_back_counter() {
     let cell = CounterCell::new();

     {
         let mut wb_counter = cell.write_back_counter();
         wb_counter.incr_delta(4);
         assert_eq!(cell.get(), 0);
+        // Multiple increments
+        wb_counter.incr_delta(2);
+        assert_eq!(cell.get(), 0);
     }

     assert_eq!(cell.get(), 6);
+    
+    // Test nested scopes
+    {
+        let mut wb_counter1 = cell.write_back_counter();
+        wb_counter1.incr_delta(10);
+        {
+            let mut wb_counter2 = cell.write_back_counter();
+            wb_counter2.incr_delta(4);
+            assert_eq!(cell.get(), 6);
+        }
+        assert_eq!(cell.get(), 10);
+    }
+    assert_eq!(cell.get(), 20);
 }

lib/segment/src/index/field_index/map_index/mutable_map_index.rs (5)

61-64: Adopting optimized counter for write operations

The change replaces direct counter increments with a write-back counter for better performance in this hot path. Good optimization for write-intensive operations.

---

71-71: Performance-optimized counter increment replaces direct call

The direct increment to the write-back counter eliminates cell access overhead during the loop, which is a good optimization for performance-critical code.

---

138-141: Adopting optimized counter for read operations

Using the write-back counter for read operations matches the pattern used in write operations, maintaining consistency and providing the same performance benefits.

---

142-143: Improved code documentation

The added comment clarifies the purpose of the overhead increment. This helps maintain code clarity while transitioning to the write-back counter pattern.

---

151-151: Consistent use of write-back counter

The counter increment inside the loop uses the write-back counter consistently with the rest of the changes, providing performance benefits for iterative operations.