Shift pointers to avoid hash collision in zend_weakmap->ht

This is the same optimization done in the previous commits for EG(weakrefs)

Author: TysonAndre

Zend/zend_weakrefs.c

@@ -63,6 +63,7 @@ typedef struct _zend_weakmap_iterator {
  */
 static zend_always_inline zend_ulong zend_object_ptr_to_weakref_key(const zend_object *object)
 {
+	ZEND_ASSERT(((uintptr_t)object) % ZEND_MM_ALIGNMENT == 0);
 	return ((uintptr_t) object) >> ZEND_MM_ALIGNMENT_LOG2;
 }
 
@@ -86,11 +87,13 @@ static inline void zend_weakref_unref_single(
 		void *ptr, uintptr_t tag, zend_object *object)
 {
 	if (tag == ZEND_WEAKREF_TAG_REF) {
+		/* Unreferencing WeakReference (at ptr) singleton that pointed to object. */
 		zend_weakref *wr = ptr;
 		wr->referent = NULL;
 	} else {
+		/* unreferencing WeakMap entry (at ptr) with a key of object. */
 		ZEND_ASSERT(tag == ZEND_WEAKREF_TAG_MAP);
-		zend_hash_index_del((HashTable *) ptr, (zend_ulong) (uintptr_t) object);
+		zend_hash_index_del((HashTable *) ptr, zend_object_ptr_to_weakref_key(object));
 	}
 }
 
@@ -132,6 +135,7 @@ static void zend_weakref_register(zend_object *object, void *payload) {
 	zend_hash_init(ht, 0, NULL, NULL, 0);
 	zend_hash_index_add_new_ptr(ht, (zend_ulong) tagged_ptr, tagged_ptr);
 	zend_hash_index_add_new_ptr(ht, (zend_ulong) payload, payload);
+	/* Replace the single WeakMap or WeakReference entry in EG(weakrefs) with a HashTable with 2 entries in place. */
 	ZVAL_PTR(zv, ZEND_WEAKREF_ENCODE(ht, ZEND_WEAKREF_TAG_HT));
 }
 
@@ -151,7 +155,7 @@ static void zend_weakref_unregister(zend_object *object, void *payload, bool wea
 		if (weakref_free) {
 			zend_weakref_unref_single(ptr, tag, object);
 		} else {
-			/* This optimization is only used in the destructor of WeakMap */
+			/* The optimization of skipping unref is only used in the destructor of WeakMap */
 			ZEND_ASSERT(ZEND_WEAKREF_GET_TAG(payload) == ZEND_WEAKREF_TAG_MAP);
 		}
 		return;
@@ -176,20 +180,21 @@ static void zend_weakref_unregister(zend_object *object, void *payload, bool wea
 		zend_weakref_unref_single(
 			ZEND_WEAKREF_GET_PTR(payload), ZEND_WEAKREF_GET_TAG(payload), object);
 	} else {
+		/* The optimization of skipping unref is only used in the destructor of WeakMap */
 		ZEND_ASSERT(ZEND_WEAKREF_GET_TAG(payload) == ZEND_WEAKREF_TAG_MAP);
 	}
 }
 
 ZEND_API zval *zend_weakrefs_hash_add(HashTable *ht, zend_object *key, zval *pData) {
-	zval *zv = zend_hash_index_add(ht, (zend_ulong) key, pData);
+	zval *zv = zend_hash_index_add(ht, zend_object_ptr_to_weakref_key(key), pData);
 	if (zv) {
 		zend_weakref_register(key, ZEND_WEAKREF_ENCODE(ht, ZEND_WEAKREF_TAG_MAP));
 	}
 	return zv;
 }
 
 ZEND_API zend_result zend_weakrefs_hash_del(HashTable *ht, zend_object *key) {
-	zval *zv = zend_hash_index_find(ht, (zend_ulong) key);
+	zval *zv = zend_hash_index_find(ht, zend_object_ptr_to_weakref_key(key));
 	if (zv) {
 		zend_weakref_unregister(key, ZEND_WEAKREF_ENCODE(ht, ZEND_WEAKREF_TAG_MAP), 1);
 		return SUCCESS;
@@ -201,6 +206,8 @@ void zend_weakrefs_init(void) {
 	zend_hash_init(&EG(weakrefs), 8, NULL, NULL, 0);
 }
 
+/* This is called when the object is garbage collected
+ * to remove all WeakReference and WeakMap entries weakly referencing that object. */
 void zend_weakrefs_notify(zend_object *object) {
 	/* Annoyingly we can't use the HT destructor here, because we need access to the key (which
 	 * is the object address), which is not provided to the dtor. */
@@ -326,13 +333,13 @@ static zend_object *zend_weakmap_create_object(zend_class_entry *ce)
 static void zend_weakmap_free_obj(zend_object *object)
 {
 	zend_weakmap *wm = zend_weakmap_from(object);
-	zend_ulong obj_addr;
-	ZEND_HASH_MAP_FOREACH_NUM_KEY(&wm->ht, obj_addr) {
+	zend_ulong obj_key;
+	ZEND_HASH_MAP_FOREACH_NUM_KEY(&wm->ht, obj_key) {
 		/* Optimization: Don't call zend_weakref_unref_single to free individual entries from wm->ht when unregistering (which would do a hash table lookup, call zend_hash_index_del, and skip over any bucket collisions).
 		 * Let freeing the corresponding values for WeakMap entries be done in zend_hash_destroy, freeing objects sequentially.
 		 * The performance difference is notable for larger WeakMaps with worse cache locality. */
 		zend_weakref_unregister(
-			(zend_object *) obj_addr, ZEND_WEAKREF_ENCODE(&wm->ht, ZEND_WEAKREF_TAG_MAP), 0);
+			zend_weakref_key_to_zend_object_ptr(obj_key), ZEND_WEAKREF_ENCODE(&wm->ht, ZEND_WEAKREF_TAG_MAP), 0);
 	} ZEND_HASH_FOREACH_END();
 	zend_hash_destroy(&wm->ht);
 	zend_object_std_dtor(&wm->std);
@@ -352,7 +359,7 @@ static zval *zend_weakmap_read_dimension(zend_object *object, zval *offset, int
 
 	zend_weakmap *wm = zend_weakmap_from(object);
 	zend_object *obj_addr = Z_OBJ_P(offset);
-	zval *zv = zend_hash_index_find(&wm->ht, (zend_ulong) obj_addr);
+	zval *zv = zend_hash_index_find(&wm->ht, zend_object_ptr_to_weakref_key(obj_addr));
 	if (zv == NULL) {
 		if (type != BP_VAR_IS) {
 			zend_throw_error(NULL,
@@ -382,9 +389,10 @@ static void zend_weakmap_write_dimension(zend_object *object, zval *offset, zval
 
 	zend_weakmap *wm = zend_weakmap_from(object);
 	zend_object *obj_addr = Z_OBJ_P(offset);
+	zend_ulong obj_key = zend_object_ptr_to_weakref_key(obj_addr);
 	Z_TRY_ADDREF_P(value);
 
-	zval *zv = zend_hash_index_find(&wm->ht, (zend_ulong) obj_addr);
+	zval *zv = zend_hash_index_find(&wm->ht, obj_key);
 	if (zv) {
 		/* Because the destructors can have side effects such as resizing or rehashing the WeakMap storage,
 		 * free the zval only after overwriting the original value. */
@@ -396,7 +404,7 @@ static void zend_weakmap_write_dimension(zend_object *object, zval *offset, zval
 	}
 
 	zend_weakref_register(obj_addr, ZEND_WEAKREF_ENCODE(&wm->ht, ZEND_WEAKREF_TAG_MAP));
-	zend_hash_index_add_new(&wm->ht, (zend_ulong) obj_addr, value);
+	zend_hash_index_add_new(&wm->ht, obj_key, value);
 }
 
 /* int return and check_empty due to Object Handler API */
@@ -408,7 +416,7 @@ static int zend_weakmap_has_dimension(zend_object *object, zval *offset, int che
 	}
 
 	zend_weakmap *wm = zend_weakmap_from(object);
-	zval *zv = zend_hash_index_find(&wm->ht, (zend_ulong) Z_OBJ_P(offset));
+	zval *zv = zend_hash_index_find(&wm->ht, zend_object_ptr_to_weakref_key(Z_OBJ_P(offset)));
 	if (!zv) {
 		return 0;
 	}
@@ -428,7 +436,7 @@ static void zend_weakmap_unset_dimension(zend_object *object, zval *offset)
 
 	zend_weakmap *wm = zend_weakmap_from(object);
 	zend_object *obj_addr = Z_OBJ_P(offset);
-	if (!zend_hash_index_exists(&wm->ht, (zend_ulong) Z_OBJ_P(offset))) {
+	if (!zend_hash_index_exists(&wm->ht, zend_object_ptr_to_weakref_key(obj_addr))) {
 		/* Object not in WeakMap, do nothing. */
 		return;
 	}
@@ -454,10 +462,10 @@ static HashTable *zend_weakmap_get_properties_for(zend_object *object, zend_prop
 	ALLOC_HASHTABLE(ht);
 	zend_hash_init(ht, zend_hash_num_elements(&wm->ht), NULL, ZVAL_PTR_DTOR, 0);
 
-	zend_ulong obj_addr;
+	zend_ulong obj_key;
 	zval *val;
-	ZEND_HASH_MAP_FOREACH_NUM_KEY_VAL(&wm->ht, obj_addr, val) {
-		zend_object *obj = (zend_object*)obj_addr;
+	ZEND_HASH_MAP_FOREACH_NUM_KEY_VAL(&wm->ht, obj_key, val) {
+		zend_object *obj = zend_weakref_key_to_zend_object_ptr(obj_key);
 		zval pair;
 		array_init(&pair);
 
@@ -491,11 +499,11 @@ static zend_object *zend_weakmap_clone_obj(zend_object *old_object)
 	zend_weakmap *new_wm = zend_weakmap_from(new_object);
 	zend_hash_copy(&new_wm->ht, &old_wm->ht, NULL);
 
-	zend_ulong obj_addr;
+	zend_ulong obj_key;
 	zval *val;
-	ZEND_HASH_MAP_FOREACH_NUM_KEY_VAL(&new_wm->ht, obj_addr, val) {
+	ZEND_HASH_MAP_FOREACH_NUM_KEY_VAL(&new_wm->ht, obj_key, val) {
 		zend_weakref_register(
-			(zend_object *) obj_addr, ZEND_WEAKREF_ENCODE(new_wm, ZEND_WEAKREF_TAG_MAP));
+			zend_weakref_key_to_zend_object_ptr(obj_key), ZEND_WEAKREF_ENCODE(new_wm, ZEND_WEAKREF_TAG_MAP));
 		zval_add_ref(val);
 	} ZEND_HASH_FOREACH_END();
 	return new_object;
@@ -542,7 +550,7 @@ static void zend_weakmap_iterator_get_current_key(zend_object_iterator *obj_iter
 		ZEND_ASSERT(0 && "Must have integer key");
 	}
 
-	ZVAL_OBJ_COPY(key, (zend_object *) num_key);
+	ZVAL_OBJ_COPY(key, zend_weakref_key_to_zend_object_ptr(num_key));
 }
 
 static void zend_weakmap_iterator_move_forward(zend_object_iterator *obj_iter)