Updated how entries are marked as occupied

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 = true;
 			ht_entry_t entry;
 
 			if (USE_SALTS) {
@@ -231,14 +233,20 @@ 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
+					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) {
 						break;
 					}
 
+					if (!entry_has_collision) {
+						break;
+					}
+
 					IncrementAndWrap(ht_offset, ht->bitmask);
 				}
 			} else {
@@ -249,11 +257,26 @@ 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;
 
 			// 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
-			row_ptr_insert_to[row_index] = entry.GetPointerOrNull();
+			// will not be compared anyway as with an empty entry we are already done. However, if we leave the loop
+			// because there is no collision, we also have no result, therefore we set the pointer to nullptr if
+			// entry_has_collision is false
+			if (USE_SALTS){
+
+				if (occupied && !salt_match && !entry_has_collision) {
+					// this is the case where we stopped because we had no collision
+					row_ptr_insert_to[row_index] = nullptr;
+					salt_match_count += 0;
+				} else {
+					// here we stopped because (a) we found an empty entry or (b) we found a matching salt
+					row_ptr_insert_to[row_index] = entry.GetPointerOrNull();
+					salt_match_count += occupied;
+				}
+			} else {
+				row_ptr_insert_to[row_index] = entry.GetPointerOrNull();
+				salt_match_count += occupied;
+			}
 		}
 
 		if (salt_match_count != 0) {
@@ -288,6 +311,7 @@ static inline void GetRowPointersInternal(DataChunk &keys, TupleDataChunkState &
 }
 
 inline bool JoinHashTable::UseSalt() const {
+	return true;
 	// only use salt for large hash tables and if there is only one equality condition as otherwise
 	// we potentially need to compare multiple keys
 	return this->capacity > USE_SALT_THRESHOLD && this->equality_predicate_columns.size() == 1;
@@ -537,15 +561,25 @@ 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);
 
+		// 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 &atomic_entry = entries[ht_offset];
+		ht_entry_t::MarkAsCollided(atomic_entry);
+
+		// increment the ht_offset of the entry
 		idx_t &ht_offset_and_salt = ht_offsets_and_salts[entry_index];
 		IncrementAndWrap(ht_offset_and_salt, capacity_mask);
 
+		// add the entry to the remaining sel vector to get processed in the next loop iteration
 		state.remaining_sel.set_index(i, entry_index);
+
 	}
 }
 
@@ -621,6 +655,7 @@ static void InsertHashesLoop(atomic<ht_entry_t> entries[], Vector &row_locations
 					break;
 				}
 
+				ht_entry_t::MarkAsCollided(atomic_entry);
 				IncrementAndWrap(ht_offset_and_salt, capacity_mask);
 			}
 

src/include/duckdb/common/types/hash.hpp

@@ -20,7 +20,12 @@ struct interval_t; // NOLINT
 // bias
 // see: https://nullprogram.com/blog/2018/07/31/
 
+inline hash_t TempMod10(uint64_t x) {
+	return x % 10;
+}
+
 inline hash_t MurmurHash64(uint64_t x) {
+	// return TempMod10(x);
 	x ^= x >> 32;
 	x *= 0xd6e8feb86659fd93U;
 	x ^= x >> 32;

src/include/duckdb/execution/ht_entry.hpp

@@ -21,8 +21,9 @@ 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;
 
@@ -37,6 +38,21 @@ struct ht_entry_t { // NOLINT
 		return value != 0;
 	}
 
+	inline bool HasCollision() const {
+		return (value & COLLISION_BIT_MASK) != 0;
+	}
+
+	inline static void MarkAsCollided(atomic<ht_entry_t> &entry) {
+
+		auto current = entry.load(std::memory_order_relaxed);
+		auto desired_value = current.value | COLLISION_BIT_MASK;
+		auto desired_entry = ht_entry_t(desired_value);
+
+		while (!entry.compare_exchange_weak(current, desired_entry, std::memory_order_relaxed)) {
+			// nothing to do here, just keep trying
+		}
+	}
+
 	// 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 +76,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