Change check for read/write handler overrides to use flags

Author: TysonAndre

ext/spl/spl_observer.c

@@ -44,19 +44,20 @@ PHPAPI zend_class_entry     *spl_ce_MultipleIterator;
 
 PHPAPI zend_object_handlers spl_handler_SplObjectStorage;
 
-typedef struct _spl_SplObjectStorage_fptrs {
-	zend_function    *get_hash;
-	bool              override_read_dimension;
-	bool              override_write_dimension;
-} spl_SplObjectStorage_fptrs;
+typedef enum {
+	SOS_OVERRIDDEN_READ_DIMENSION = 1,
+	SOS_OVERRIDDEN_WRITE_DIMENSION = 2,
+} SplObjectStorageBitFlags;
 
 typedef struct _spl_SplObjectStorage { /* {{{ */
-	HashTable                   storage;
-	zend_long                   index;
-	HashPosition                pos;
-	zend_long                   flags;
-	spl_SplObjectStorage_fptrs *fptrs;
-	zend_object                 std;
+	HashTable         storage;
+	zend_long         index;
+	HashPosition      pos;
+	/* In SplObjectStorage, flags is a hidden implementation detail to optimize ArrayAccess handlers.
+	 * In MultipleIterator on a different class hierarchy, flags is a user settable value controlling iteration behavior. */
+	zend_long         flags;
+	zend_function    *fptr_get_hash;
+	zend_object       std;
 } spl_SplObjectStorage; /* }}} */
 
 /* {{{ storage is an assoc array of [zend_object*]=>[zval *obj, zval *inf] */
@@ -79,19 +80,15 @@ void spl_SplObjectStorage_free_storage(zend_object *object) /* {{{ */
 	zend_object_std_dtor(&intern->std);
 
 	zend_hash_destroy(&intern->storage);
-
-	if (UNEXPECTED(intern->fptrs)) {
-		efree_size(intern->fptrs, sizeof(spl_SplObjectStorage_fptrs));
-	}
 } /* }}} */
 
 static int spl_object_storage_get_hash(zend_hash_key *key, spl_SplObjectStorage *intern, zend_object *obj) {
-	if (UNEXPECTED(intern->fptrs) && intern->fptrs->get_hash) {
+	if (UNEXPECTED(intern->fptr_get_hash)) {
 		zval param;
 		zval rv;
 		ZVAL_OBJ(&param, obj);
 		zend_call_method_with_1_params(
-			&intern->std, intern->std.ce, &intern->fptrs->get_hash, "getHash", &rv, &param);
+			&intern->std, intern->std.ce, &intern->fptr_get_hash, "getHash", &rv, &param);
 		if (!Z_ISUNDEF(rv)) {
 			if (Z_TYPE(rv) == IS_STRING) {
 				key->key = Z_STR(rv);
@@ -154,7 +151,7 @@ static spl_SplObjectStorageElement *spl_object_storage_attach_handle(spl_SplObje
 	zval *entry_zv;
 	spl_SplObjectStorageElement *pelement;
 	entry_zv = zend_hash_index_lookup(&intern->storage, handle);
-	ZEND_ASSERT(intern->fptrs == NULL || !intern->fptrs->override_write_dimension);
+	ZEND_ASSERT(!(intern->flags & SOS_OVERRIDDEN_WRITE_DIMENSION));
 
 	if (Z_TYPE_P(entry_zv) != IS_NULL) {
 		zval zv_inf;
@@ -179,7 +176,7 @@ static spl_SplObjectStorageElement *spl_object_storage_attach_handle(spl_SplObje
 
 spl_SplObjectStorageElement *spl_object_storage_attach(spl_SplObjectStorage *intern, zend_object *obj, zval *inf) /* {{{ */
 {
-	if (EXPECTED(intern->fptrs == NULL || !intern->fptrs->override_write_dimension)) {
+	if (EXPECTED(!(intern->flags & SOS_OVERRIDDEN_WRITE_DIMENSION))) {
 		return spl_object_storage_attach_handle(intern, obj, inf);
 	}
 
@@ -269,21 +266,22 @@ static zend_object *spl_object_storage_new_ex(zend_class_entry *class_type, zend
 
 	while (parent) {
 		if (parent == spl_ce_SplObjectStorage) {
-			/* TODO: Make these singletons with a map from class entry to IS_NULL/IS_PTR */
+			/* Possible optimization: Cache these results with a map from class entry to IS_NULL/IS_PTR.
+			 * Or maybe just a single item with the result for the most recently loaded subclass. */
 			if (class_type != spl_ce_SplObjectStorage) {
-				spl_SplObjectStorage_fptrs fptrs;
-				fptrs.get_hash = zend_hash_str_find_ptr(&class_type->function_table, "gethash", sizeof("gethash") - 1);
-				if (fptrs.get_hash->common.scope == spl_ce_SplObjectStorage) {
-					fptrs.get_hash = NULL;
+				zend_function *get_hash = zend_hash_str_find_ptr(&class_type->function_table, "gethash", sizeof("gethash") - 1);
+				if (get_hash->common.scope != spl_ce_SplObjectStorage) {
+					intern->fptr_get_hash = get_hash;
 				}
-				fptrs.override_read_dimension = fptrs.get_hash != NULL ||
+				if (get_hash != NULL ||
 					SPL_OBJECT_STORAGE_CLASS_HAS_OVERRIDE(class_type, "offsetget") ||
-					SPL_OBJECT_STORAGE_CLASS_HAS_OVERRIDE(class_type, "offsetexists");
+					SPL_OBJECT_STORAGE_CLASS_HAS_OVERRIDE(class_type, "offsetexists")) {
+					intern->flags |= SOS_OVERRIDDEN_READ_DIMENSION;
+				}
 
-				fptrs.override_write_dimension = fptrs.get_hash != NULL || SPL_OBJECT_STORAGE_CLASS_HAS_OVERRIDE(class_type, "offsetset");
-				if (fptrs.get_hash || fptrs.override_read_dimension || fptrs.override_write_dimension) {
-					intern->fptrs = emalloc(sizeof(spl_SplObjectStorage_fptrs));
-					*intern->fptrs = fptrs;
+				if (get_hash != NULL ||
+					SPL_OBJECT_STORAGE_CLASS_HAS_OVERRIDE(class_type, "offsetset")) {
+					intern->flags |= SOS_OVERRIDDEN_WRITE_DIMENSION;
 				}
 			}
 			break;
@@ -437,7 +435,7 @@ PHP_METHOD(SplObjectStorage, attach)
 static zval *spl_object_storage_read_dimension(zend_object *object, zval *offset, int type, zval *rv)
 {
 	spl_SplObjectStorage *intern = spl_object_storage_from_obj(object);
-	if (UNEXPECTED(offset == NULL || Z_TYPE_P(offset) != IS_OBJECT || (intern->fptrs && intern->fptrs->override_read_dimension))) {
+	if (UNEXPECTED(offset == NULL || Z_TYPE_P(offset) != IS_OBJECT || (intern->flags & SOS_OVERRIDDEN_READ_DIMENSION))) {
 		/* Can't optimize it if getHash, offsetExists, or offsetGet is overridden */
 		return zend_std_read_dimension(object, offset, type, rv);
 	}
@@ -460,7 +458,7 @@ static zval *spl_object_storage_read_dimension(zend_object *object, zval *offset
 static void spl_object_storage_write_dimension(zend_object *object, zval *offset, zval *inf)
 {
 	spl_SplObjectStorage *intern = spl_object_storage_from_obj(object);
-	if (UNEXPECTED(offset == NULL || Z_TYPE_P(offset) != IS_OBJECT || (intern->fptrs && intern->fptrs->override_write_dimension))) {
+	if (UNEXPECTED(offset == NULL || Z_TYPE_P(offset) != IS_OBJECT || (intern->flags & SOS_OVERRIDDEN_WRITE_DIMENSION))) {
 		zend_std_write_dimension(object, offset, inf);
 		return;
 	}