use part of salt as collision bit

src/execution/join_hashtable.cpp

@@ -223,6 +223,8 @@ static inline void GetRowPointersInternal(DataChunk &keys, TupleDataChunkState &
 
 			idx_t &ht_offset = ht_offsets[row_index];
 			bool occupied;
+			bool salt_match = true;
+			bool entry_has_collision;
 			ht_entry_t entry;
 
 			if (USE_SALTS) {
@@ -231,11 +233,13 @@ static inline void GetRowPointersInternal(DataChunk &keys, TupleDataChunkState &
 				while (true) {
 					entry = entries[ht_offset];
 					occupied = entry.IsOccupied();
-					bool salt_match = entry.GetSalt() == row_salt;
+					salt_match = entry.GetSalt() == row_salt;
 
-					// condition for incrementing the ht_offset: occupied and row_salt does not match -> move to next
-					// entry
-					if (!occupied || salt_match) {
+					entry_has_collision = entry.HasCollision();
+
+					// condition for incrementing the ht_offset: occupied and salt does not match and entry has collision
+					// reverse the condition to break out of the loop
+					if (!occupied || salt_match || !entry_has_collision) {
 						break;
 					}
 
@@ -249,7 +253,7 @@ static inline void GetRowPointersInternal(DataChunk &keys, TupleDataChunkState &
 			// the entries we need to process in the next iteration are the ones that are occupied and the row_salt
 			// does not match, the ones that are empty need no further processing
 			state.salt_match_sel.set_index(salt_match_count, row_index);
-			salt_match_count += occupied;
+			salt_match_count += occupied && salt_match;
 
 			// entry might be empty, so the pointer in the entry is nullptr, but this does not matter as the row
 			// will not be compared anyway as with an empty entry we are already done
@@ -537,7 +541,8 @@ static inline void InsertMatchesAndIncrementMisses(atomic<ht_entry_t> entries[],
 		InsertRowToEntry<PARALLEL, false>(entry, row_ptr_to_insert, salt, ht.pointer_offset);
 	}
 
-	// Linear probing: each of the entries that do not match move to the next entry in the HT
+	// Linear probing: each of the entries that do not match move to the next entry in the HT, also we mark them with
+	// the collision bit
 	for (idx_t i = 0; i < key_no_match_count; i++) {
 		const auto need_compare_idx = state.key_no_match_sel.get_index(i);
 		const auto entry_index = state.salt_match_sel.get_index(need_compare_idx);
@@ -546,6 +551,12 @@ static inline void InsertMatchesAndIncrementMisses(atomic<ht_entry_t> entries[],
 		IncrementAndWrap(ht_offset_and_salt, capacity_mask);
 
 		state.remaining_sel.set_index(i, entry_index);
+
+		// mark the entry as collided, we don't need to care about thread synchronisation as the mark is an OR operation
+		// and the worst case is that we mark the same entry multiple times
+		const auto &ht_offset = ht_offsets_and_salts[entry_index] & ht_entry_t::POINTER_MASK;
+		auto &entry = entries[ht_offset];
+		entry.load(std::memory_order_relaxed).MarkAsCollided();
 	}
 }
 

src/include/duckdb/execution/ht_entry.hpp

@@ -21,11 +21,15 @@ namespace duckdb {
 */
 struct ht_entry_t { // NOLINT
 public:
+
+	static constexpr const hash_t COLLISION_BIT_MASK = 0x8000000000000000;
 	//! Upper 16 bits are salt
-	static constexpr const hash_t SALT_MASK = 0xFFFF000000000000;
+	static constexpr const hash_t SALT_MASK = 0x7FFF000000000000;
 	//! Lower 48 bits are the pointer
 	static constexpr const hash_t POINTER_MASK = 0x0000FFFFFFFFFFFF;
 
+
+
 	explicit inline ht_entry_t(hash_t value_p) noexcept : value(value_p) {
 	}
 
@@ -37,6 +41,15 @@ struct ht_entry_t { // NOLINT
 		return value != 0;
 	}
 
+	inline bool HasCollision() const {
+        return (value & COLLISION_BIT_MASK) != 0;
+    }
+
+	inline bool MarkAsCollided() {
+        value |= COLLISION_BIT_MASK;
+        return true;
+    }
+
 	// Returns a pointer based on the stored value without checking cell occupancy.
 	// This can return a nullptr if the cell is not occupied.
 	inline data_ptr_t GetPointerOrNull() const {
@@ -60,7 +73,7 @@ struct ht_entry_t { // NOLINT
 
 	// Returns the salt, leaves upper salt bits intact, sets lower bits to all 1's
 	static inline hash_t ExtractSalt(hash_t hash) {
-		return hash | POINTER_MASK;
+		return hash | ~SALT_MASK;
 	}
 
 	// Returns the salt, leaves upper salt bits intact, sets lower bits to all 0's