Changed zend_ast_ref structure to use only one allocation, removing dichotomy between heap/arena ASTs.

Author: dstogov

UPGRADING.INTERNALS

@@ -13,6 +13,7 @@ PHP 7.2 INTERNALS UPGRADE NOTES
   j. Run-time constant operand addressing
   k. Array/Object recursion protection
   l. HASH_FLAG_PERSISTENT
+  m. zend_ast_ref
 
 2. Build system changes
   a. Unix build system changes
@@ -88,6 +89,11 @@ PHP 7.2 INTERNALS UPGRADE NOTES
   l. HASH_FLAG_PERSISTENT renamed into IS_ARRAY_PERSISTENT and moved into
      GC_FLAGS (to be consistent with IS_STR_PERSISTENT).
 
+  m. zend_ast_ref structure is changed to use only one allocation.
+     zend_ast_copy() now returns zend_ast_ref (instead of zend_asr).
+     zend_ast_destroy_and_free() is removed. ZVAL_NEW_AST() is replaced
+     by ZVAL_AST().
+
 ========================
 2. Build system changes
 ========================

Zend/zend_ast.c

@@ -451,40 +451,92 @@ ZEND_API int zend_ast_evaluate(zval *result, zend_ast *ast, zend_class_entry *sc
 	return ret;
 }
 
-ZEND_API zend_ast *zend_ast_copy(zend_ast *ast)
+static size_t zend_ast_tree_size(zend_ast *ast)
 {
-	if (ast == NULL) {
-		return NULL;
-	} else if (ast->kind == ZEND_AST_ZVAL) {
-		zend_ast_zval *new = emalloc(sizeof(zend_ast_zval));
+	size_t size;
+
+	if (ast->kind == ZEND_AST_ZVAL) {
+		size = sizeof(zend_ast_zval);
+	} else if (zend_ast_is_list(ast)) {
+		uint32_t i;
+		zend_ast_list *list = zend_ast_get_list(ast);
+
+		size = zend_ast_list_size(list->children);
+		for (i = 0; i < list->children; i++) {
+			if (list->child[i]) {
+				size += zend_ast_tree_size(list->child[i]);
+			}
+		}
+	} else {
+		uint32_t i, children = zend_ast_get_num_children(ast);
+
+		size = zend_ast_size(children);
+		for (i = 0; i < children; i++) {
+			if (ast->child[i]) {
+				size += zend_ast_tree_size(ast->child[i]);
+			}
+		}
+	}
+	return size;
+}
+
+static void* zend_ast_tree_copy(zend_ast *ast, void *buf)
+{
+	if (ast->kind == ZEND_AST_ZVAL) {
+		zend_ast_zval *new = (zend_ast_zval*)buf;
 		new->kind = ZEND_AST_ZVAL;
 		new->attr = ast->attr;
 		ZVAL_COPY(&new->val, zend_ast_get_zval(ast));
-		return (zend_ast *) new;
+		buf = (void*)((char*)buf + sizeof(zend_ast_zval));
 	} else if (zend_ast_is_list(ast)) {
 		zend_ast_list *list = zend_ast_get_list(ast);
-		zend_ast_list *new = emalloc(zend_ast_list_size(list->children));
+		zend_ast_list *new = (zend_ast_list*)buf;
 		uint32_t i;
 		new->kind = list->kind;
 		new->attr = list->attr;
 		new->children = list->children;
+		buf = (void*)((char*)buf + zend_ast_list_size(list->children));
 		for (i = 0; i < list->children; i++) {
-			new->child[i] = zend_ast_copy(list->child[i]);
+			if (list->child[i]) {
+				new->child[i] = (zend_ast*)buf;
+				buf = zend_ast_tree_copy(list->child[i], buf);
+			} else {
+				new->child[i] = NULL;
+			}
 		}
-		return (zend_ast *) new;
 	} else {
 		uint32_t i, children = zend_ast_get_num_children(ast);
-		zend_ast *new = emalloc(zend_ast_size(children));
+		zend_ast *new = (zend_ast*)buf;
 		new->kind = ast->kind;
 		new->attr = ast->attr;
+		buf = (void*)((char*)buf + zend_ast_size(children));
 		for (i = 0; i < children; i++) {
-			new->child[i] = zend_ast_copy(ast->child[i]);
+			if (ast->child[i]) {
+				new->child[i] = (zend_ast*)buf;
+				buf = zend_ast_tree_copy(ast->child[i], buf);
+			} else {
+				new->child[i] = NULL;
+			}
 		}
-		return new;
 	}
+	return buf;
 }
 
-static void zend_ast_destroy_ex(zend_ast *ast, zend_bool free) {
+ZEND_API zend_ast_ref *zend_ast_copy(zend_ast *ast)
+{
+	size_t tree_size;
+	zend_ast_ref *ref;
+
+	ZEND_ASSERT(ast != NULL);
+	tree_size = zend_ast_tree_size(ast) + sizeof(zend_ast_ref);
+	ref = emalloc(tree_size);
+	zend_ast_tree_copy(ast, GC_AST(ref));
+	GC_REFCOUNT(ref) = 1;
+	GC_TYPE_INFO(ref) = IS_CONSTANT_AST;
+	return ref;
+}
+
+ZEND_API void zend_ast_destroy(zend_ast *ast) {
 	if (!ast) {
 		return;
 	}
@@ -508,37 +560,26 @@ static void zend_ast_destroy_ex(zend_ast *ast, zend_bool free) {
 			if (decl->doc_comment) {
 				zend_string_release(decl->doc_comment);
 			}
-			zend_ast_destroy_ex(decl->child[0], free);
-			zend_ast_destroy_ex(decl->child[1], free);
-			zend_ast_destroy_ex(decl->child[2], free);
-			zend_ast_destroy_ex(decl->child[3], free);
+			zend_ast_destroy(decl->child[0]);
+			zend_ast_destroy(decl->child[1]);
+			zend_ast_destroy(decl->child[2]);
+			zend_ast_destroy(decl->child[3]);
 			break;
 		}
 		default:
 			if (zend_ast_is_list(ast)) {
 				zend_ast_list *list = zend_ast_get_list(ast);
 				uint32_t i;
 				for (i = 0; i < list->children; i++) {
-					zend_ast_destroy_ex(list->child[i], free);
+					zend_ast_destroy(list->child[i]);
 				}
 			} else {
 				uint32_t i, children = zend_ast_get_num_children(ast);
 				for (i = 0; i < children; i++) {
-					zend_ast_destroy_ex(ast->child[i], free);
+					zend_ast_destroy(ast->child[i]);
 				}
 			}
 	}
-
-	if (free) {
-		efree(ast);
-	}
-}
-
-ZEND_API void zend_ast_destroy(zend_ast *ast) {
-	zend_ast_destroy_ex(ast, 0);
-}
-ZEND_API void zend_ast_destroy_and_free(zend_ast *ast) {
-	zend_ast_destroy_ex(ast, 1);
 }
 
 ZEND_API void zend_ast_apply(zend_ast *ast, zend_ast_apply_func fn) {

Zend/zend_ast.h

@@ -206,9 +206,8 @@ ZEND_API zend_ast *zend_ast_list_add(zend_ast *list, zend_ast *op);
 ZEND_API int zend_ast_evaluate(zval *result, zend_ast *ast, zend_class_entry *scope);
 ZEND_API zend_string *zend_ast_export(const char *prefix, zend_ast *ast, const char *suffix);
 
-ZEND_API zend_ast *zend_ast_copy(zend_ast *ast);
+ZEND_API zend_ast_ref *zend_ast_copy(zend_ast *ast);
 ZEND_API void zend_ast_destroy(zend_ast *ast);
-ZEND_API void zend_ast_destroy_and_free(zend_ast *ast);
 
 typedef void (*zend_ast_apply_func)(zend_ast **ast_ptr);
 ZEND_API void zend_ast_apply(zend_ast *ast, zend_ast_apply_func fn);

Zend/zend_compile.c

@@ -8040,7 +8040,7 @@ void zend_const_expr_to_zval(zval *result, zend_ast *ast) /* {{{ */
 	if (ast->kind == ZEND_AST_ZVAL) {
 		ZVAL_COPY_VALUE(result, zend_ast_get_zval(ast));
 	} else {
-		ZVAL_NEW_AST(result, zend_ast_copy(ast));
+		ZVAL_AST(result, zend_ast_copy(ast));
 		/* destroy the ast here, it might have been replaced */
 		zend_ast_destroy(ast);
 	}

Zend/zend_types.h

@@ -355,7 +355,7 @@ struct _zend_reference {
 
 struct _zend_ast_ref {
 	zend_refcounted_h gc;
-	zend_ast         *ast;
+	/*zend_ast        ast; zend_ast follows the zend_ast_ref structure */
 };
 
 /* regular data types */
@@ -635,7 +635,9 @@ static zend_always_inline zend_uchar zval_get_type(const zval* pz) {
 #define Z_AST(zval)					(zval).value.ast
 #define Z_AST_P(zval_p)				Z_AST(*(zval_p))
 
-#define Z_ASTVAL(zval)				(zval).value.ast->ast
+#define GC_AST(p)					((zend_ast*)(((char*)p) + sizeof(zend_ast_ref)))
+
+#define Z_ASTVAL(zval)				GC_AST(Z_AST(zval))
 #define Z_ASTVAL_P(zval_p)			Z_ASTVAL(*(zval_p))
 
 #define Z_INDIRECT(zval)			(zval).value.zv
@@ -818,14 +820,9 @@ static zend_always_inline zend_uchar zval_get_type(const zval* pz) {
 		Z_TYPE_INFO_P(z) = IS_REFERENCE_EX;						\
 	} while (0)
 
-#define ZVAL_NEW_AST(z, a) do {									\
+#define ZVAL_AST(z, ast) do {									\
 		zval *__z = (z);										\
-		zend_ast_ref *_ast =									\
-		(zend_ast_ref *) emalloc(sizeof(zend_ast_ref));			\
-		GC_REFCOUNT(_ast) = 1;									\
-		GC_TYPE_INFO(_ast) = IS_CONSTANT_AST;					\
-		_ast->ast = (a);										\
-		Z_AST_P(__z) = _ast;									\
+		Z_AST_P(__z) = ast;										\
 		Z_TYPE_INFO_P(__z) = IS_CONSTANT_AST_EX;				\
 	} while (0)
 

Zend/zend_variables.c

@@ -46,8 +46,8 @@ ZEND_API void ZEND_FASTCALL _zval_dtor_func(zend_refcounted *p ZEND_FILE_LINE_DC
 		case IS_CONSTANT_AST: {
 				zend_ast_ref *ast = (zend_ast_ref*)p;
 
-				zend_ast_destroy_and_free(ast->ast);
-				efree_size(ast, sizeof(zend_ast_ref));
+				zend_ast_destroy(GC_AST(ast));
+				efree(ast);
 				break;
 			}
 		case IS_OBJECT: {
@@ -174,8 +174,7 @@ ZEND_API void ZEND_FASTCALL _zval_copy_ctor_func(zval *zvalue ZEND_FILE_LINE_DC)
 		CHECK_ZVAL_STRING_REL(Z_STR_P(zvalue));
 		Z_STR_P(zvalue) = zend_string_dup(Z_STR_P(zvalue), 0);
 	} else if (EXPECTED(Z_TYPE_P(zvalue) == IS_CONSTANT_AST)) {
-		zend_ast *copy = zend_ast_copy(Z_ASTVAL_P(zvalue));
-		ZVAL_NEW_AST(zvalue, copy);
+		ZVAL_AST(zvalue, zend_ast_copy(Z_ASTVAL_P(zvalue)));
 	}
 }
 

ext/opcache/zend_file_cache.c

@@ -337,9 +337,7 @@ static void zend_file_cache_serialize_zval(zval                     *zv,
 				SERIALIZE_PTR(Z_AST_P(zv));
 				ast = Z_AST_P(zv);
 				UNSERIALIZE_PTR(ast);
-				if (!IS_SERIALIZED(ast->ast)) {
-					ast->ast = zend_file_cache_serialize_ast(ast->ast, script, info, buf);
-				}
+				zend_file_cache_serialize_ast(Z_ASTVAL_P(zv), script, info, buf);
 			}
 			break;
 	}
@@ -950,10 +948,7 @@ static void zend_file_cache_unserialize_zval(zval                    *zv,
 				zend_ast_ref *ast;
 
 				UNSERIALIZE_PTR(Z_AST_P(zv));
-				ast = Z_AST_P(zv);
-				if (!IS_UNSERIALIZED(ast->ast)) {
-					ast->ast = zend_file_cache_unserialize_ast(ast->ast, script, buf);
-				}
+				zend_file_cache_unserialize_ast(Z_ASTVAL_P(zv), script, buf);
 			}
 			break;
 	}

ext/opcache/zend_persist.c

@@ -273,7 +273,6 @@ static zend_ast *zend_persist_ast(zend_ast *ast)
 		}
 	}
 
-	efree(ast);
 	return node;
 }
 
@@ -323,10 +322,12 @@ static void zend_persist_zval(zval *z)
 				Z_AST_P(z) = new_ptr;
 				Z_TYPE_FLAGS_P(z) = IS_TYPE_CONSTANT | IS_TYPE_COPYABLE;
 			} else {
-				zend_accel_store(Z_AST_P(z), sizeof(zend_ast_ref));
-				Z_ASTVAL_P(z) = zend_persist_ast(Z_ASTVAL_P(z));
+				zend_ast_ref *old_ref = Z_AST_P(z);
+				Z_ARR_P(z) = zend_accel_memdup(Z_AST_P(z), sizeof(zend_ast_ref));
+				zend_persist_ast(GC_AST(old_ref));
 				Z_TYPE_FLAGS_P(z) = IS_TYPE_CONSTANT | IS_TYPE_COPYABLE;
 				GC_REFCOUNT(Z_COUNTED_P(z)) = 2;
+				efree(old_ref);
 			}
 			break;
 	}