Add helper APIs for get_gc implementations

get_gc() implementations that need to explore heterogeneous data
currently work by computing how many GC entries they need,
allocating a buffer for that and storing it on the object. This
is inefficient and wastes memory, because the buffer is retained
after the GC run.

This commit adds an API for a single global GC buffer, which can
be reused by get_gc implementations (as only one get_gc call is
ever active at the same time). The GC buffer will automatically
grow during the GC run and be discarded at the end.

8 files changed, 89 insertions, 113 deletions

diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c
index 1c7c904b19..ce801c3098 100644
--- a/Zend/zend_execute_API.c
+++ b/Zend/zend_execute_API.c
@@ -184,6 +184,8 @@ void init_executor(void) /* {{{ */
 	EG(persistent_functions_count) = EG(function_table)->nNumUsed;
 	EG(persistent_classes_count)   = EG(class_table)->nNumUsed;
 
+	EG(get_gc_buffer).start = EG(get_gc_buffer).end = EG(get_gc_buffer).cur = NULL;
+
 	zend_weakrefs_init();
 
 	EG(active) = 1;
diff --git a/Zend/zend_gc.c b/Zend/zend_gc.c
index 30698f4484..76f60b7abf 100644
--- a/Zend/zend_gc.c
+++ b/Zend/zend_gc.c
@@ -1416,6 +1416,8 @@ tail_call:
 	} while (0);
 }
 
+static void zend_get_gc_buffer_release();
+
 ZEND_API int zend_gc_collect_cycles(void)
 {
 	int count = 0;
@@ -1451,6 +1453,7 @@ ZEND_API int zend_gc_collect_cycles(void)
 		if (!GC_G(num_roots)) {
 			/* nothing to free */
 			GC_TRACE("Nothing to free");
+			zend_get_gc_buffer_release();
 			GC_G(gc_active) = 0;
 			return 0;
 		}
@@ -1533,6 +1536,7 @@ ZEND_API int zend_gc_collect_cycles(void)
 
 			if (GC_G(gc_protected)) {
 				/* something went wrong */
+				zend_get_gc_buffer_release();
 				return 0;
 			}
 		}
@@ -1595,7 +1599,7 @@ ZEND_API int zend_gc_collect_cycles(void)
 	}
 
 	gc_compact();
-
+	zend_get_gc_buffer_release();
 	return count;
 }
 
@@ -1607,6 +1611,28 @@ ZEND_API void zend_gc_get_status(zend_gc_status *status)
 	status->num_roots = GC_G(num_roots);
 }
 
+ZEND_API zend_get_gc_buffer *zend_get_gc_buffer_create() {
+	/* There can only be one get_gc() call active at a time,
+	 * so there only needs to be one buffer. */
+	zend_get_gc_buffer *gc_buffer = &EG(get_gc_buffer);
+	gc_buffer->cur = gc_buffer->start;
+	return gc_buffer;
+}
+
+ZEND_API void zend_get_gc_buffer_grow(zend_get_gc_buffer *gc_buffer) {
+	size_t old_capacity = gc_buffer->end - gc_buffer->start;
+	size_t new_capacity = old_capacity == 0 ? 64 : old_capacity * 2;
+	gc_buffer->start = erealloc(gc_buffer->start, new_capacity * sizeof(zval));
+	gc_buffer->end = gc_buffer->start + new_capacity;
+	gc_buffer->cur = gc_buffer->start + old_capacity;
+}
+
+static void zend_get_gc_buffer_release() {
+	zend_get_gc_buffer *gc_buffer = &EG(get_gc_buffer);
+	efree(gc_buffer->start);
+	gc_buffer->start = gc_buffer->end = gc_buffer->cur = NULL;
+}
+
 #ifdef ZTS
 size_t zend_gc_globals_size(void)
 {
diff --git a/Zend/zend_gc.h b/Zend/zend_gc.h
index d7b4e1a533..4754f31713 100644
--- a/Zend/zend_gc.h
+++ b/Zend/zend_gc.h
@@ -84,4 +84,43 @@ static zend_always_inline void gc_check_possible_root(zend_refcounted *ref)
 	}
 }
 
+/* These APIs can be used to simplify object get_gc implementations
+ * over heterogenous structures. See zend_generator_get_gc() for
+ * a usage example. */
+
+typedef struct {
+	zval *cur;
+	zval *end;
+	zval *start;
+} zend_get_gc_buffer;
+
+ZEND_API zend_get_gc_buffer *zend_get_gc_buffer_create();
+ZEND_API void zend_get_gc_buffer_grow(zend_get_gc_buffer *gc_buffer);
+
+static zend_always_inline void zend_get_gc_buffer_add_zval(
+		zend_get_gc_buffer *gc_buffer, zval *zv) {
+	if (Z_REFCOUNTED_P(zv)) {
+		if (UNEXPECTED(gc_buffer->cur == gc_buffer->end)) {
+			zend_get_gc_buffer_grow(gc_buffer);
+		}
+		ZVAL_COPY_VALUE(gc_buffer->cur, zv);
+		gc_buffer->cur++;
+	}
+}
+
+static zend_always_inline void zend_get_gc_buffer_add_obj(
+		zend_get_gc_buffer *gc_buffer, zend_object *obj) {
+	if (UNEXPECTED(gc_buffer->cur == gc_buffer->end)) {
+		zend_get_gc_buffer_grow(gc_buffer);
+	}
+	ZVAL_OBJ(gc_buffer->cur, obj);
+	gc_buffer->cur++;
+}
+
+static zend_always_inline void zend_get_gc_buffer_use(
+		zend_get_gc_buffer *gc_buffer, zval **table, int *n) {
+	*table = gc_buffer->start;
+	*n = gc_buffer->cur - gc_buffer->start;
+}
+
 #endif /* ZEND_GC_H */
diff --git a/Zend/zend_generators.c b/Zend/zend_generators.c
index b3261842eb..ce681557df 100644
--- a/Zend/zend_generators.c
+++ b/Zend/zend_generators.c
@@ -152,12 +152,6 @@ ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished
 			OBJ_RELEASE(ZEND_CLOSURE_OBJECT(EX(func)));
 		}
 
-		/* Free GC buffer. GC for closed generators doesn't need an allocated buffer */
-		if (generator->gc_buffer) {
-			efree(generator->gc_buffer);
-			generator->gc_buffer = NULL;
-		}
-
 		efree(execute_data);
 	}
 }
@@ -279,63 +273,11 @@ static void zend_generator_free_storage(zend_object *object) /* {{{ */
 }
 /* }}} */
 
-static uint32_t calc_gc_buffer_size(zend_generator *generator) /* {{{ */
-{
-	uint32_t size = 4; /* value, key, retval, values */
-	if (generator->execute_data) {
-		zend_execute_data *execute_data = generator->execute_data;
-		zend_op_array *op_array = &EX(func)->op_array;
-
-		/* Compiled variables */
-		if (!(EX_CALL_INFO() & ZEND_CALL_HAS_SYMBOL_TABLE)) {
-			size += op_array->last_var;
-		}
-		/* Extra args */
-		if (EX_CALL_INFO() & ZEND_CALL_FREE_EXTRA_ARGS) {
-			size += EX_NUM_ARGS() - op_array->num_args;
-		}
-		size += (EX_CALL_INFO() & ZEND_CALL_RELEASE_THIS) != 0; /* $this */
-		size += (EX_CALL_INFO() & ZEND_CALL_CLOSURE) != 0; /* Closure object */
-
-		/* Live vars */
-		if (execute_data->opline != op_array->opcodes) {
-			/* -1 required because we want the last run opcode, not the next to-be-run one. */
-			uint32_t i, op_num = execute_data->opline - op_array->opcodes - 1;
-			for (i = 0; i < op_array->last_live_range; i++) {
-				const zend_live_range *range = &op_array->live_range[i];
-				if (range->start > op_num) {
-					/* Further ranges will not be relevant... */
-					break;
-				} else if (op_num < range->end) {
-					/* LIVE_ROPE and LIVE_SILENCE not relevant for GC */
-					uint32_t kind = range->var & ZEND_LIVE_MASK;
-					if (kind == ZEND_LIVE_TMPVAR || kind == ZEND_LIVE_LOOP) {
-						size++;
-					}
-				}
-			}
-		}
-
-		/* Yield from root references */
-		if (generator->node.children == 0) {
-			zend_generator *root = generator->node.ptr.root;
-			while (root != generator) {
-				root = zend_generator_get_child(&root->node, generator);
-				size++;
-			}
-		}
-	}
-	return size;
-}
-/* }}} */
-
 static HashTable *zend_generator_get_gc(zend_object *object, zval **table, int *n) /* {{{ */
 {
 	zend_generator *generator = (zend_generator*)object;
 	zend_execute_data *execute_data = generator->execute_data;
 	zend_op_array *op_array;
-	zval *gc_buffer;
-	uint32_t gc_buffer_size;
 
 	if (!execute_data) {
 		/* If the generator has been closed, it can only hold on to three values: The value, key
@@ -346,24 +288,17 @@ static HashTable *zend_generator_get_gc(zend_object *object, zval **table, int *
 	}
 
 	op_array = &EX(func)->op_array;
-	gc_buffer_size = calc_gc_buffer_size(generator);
-	if (generator->gc_buffer_size < gc_buffer_size) {
-		generator->gc_buffer = safe_erealloc(generator->gc_buffer, sizeof(zval), gc_buffer_size, 0);
-		generator->gc_buffer_size = gc_buffer_size;
-	}
-
-	*n = gc_buffer_size;
-	*table = gc_buffer = generator->gc_buffer;
 
-	ZVAL_COPY_VALUE(gc_buffer++, &generator->value);
-	ZVAL_COPY_VALUE(gc_buffer++, &generator->key);
-	ZVAL_COPY_VALUE(gc_buffer++, &generator->retval);
-	ZVAL_COPY_VALUE(gc_buffer++, &generator->values);
+	zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create();
+	zend_get_gc_buffer_add_zval(gc_buffer, &generator->value);
+	zend_get_gc_buffer_add_zval(gc_buffer, &generator->key);
+	zend_get_gc_buffer_add_zval(gc_buffer, &generator->retval);
+	zend_get_gc_buffer_add_zval(gc_buffer, &generator->values);
 
 	if (!(EX_CALL_INFO() & ZEND_CALL_HAS_SYMBOL_TABLE)) {
 		uint32_t i, num_cvs = EX(func)->op_array.last_var;
 		for (i = 0; i < num_cvs; i++) {
-			ZVAL_COPY_VALUE(gc_buffer++, EX_VAR_NUM(i));
+			zend_get_gc_buffer_add_zval(gc_buffer, EX_VAR_NUM(i));
 		}
 	}
 
@@ -371,15 +306,15 @@ static HashTable *zend_generator_get_gc(zend_object *object, zval **table, int *
 		zval *zv = EX_VAR_NUM(op_array->last_var + op_array->T);
 		zval *end = zv + (EX_NUM_ARGS() - op_array->num_args);
 		while (zv != end) {
-			ZVAL_COPY_VALUE(gc_buffer++, zv++);
+			zend_get_gc_buffer_add_zval(gc_buffer, zv++);
 		}
 	}
 
 	if (EX_CALL_INFO() & ZEND_CALL_RELEASE_THIS) {
-		ZVAL_OBJ(gc_buffer++, Z_OBJ(execute_data->This));
+		zend_get_gc_buffer_add_obj(gc_buffer, Z_OBJ(execute_data->This));
 	}
 	if (EX_CALL_INFO() & ZEND_CALL_CLOSURE) {
-		ZVAL_OBJ(gc_buffer++, ZEND_CLOSURE_OBJECT(EX(func)));
+		zend_get_gc_buffer_add_obj(gc_buffer, ZEND_CLOSURE_OBJECT(EX(func)));
 	}
 
 	if (execute_data->opline != op_array->opcodes) {
@@ -393,7 +328,7 @@ static HashTable *zend_generator_get_gc(zend_object *object, zval **table, int *
 				uint32_t var_num = range->var & ~ZEND_LIVE_MASK;
 				zval *var = EX_VAR(var_num);
 				if (kind == ZEND_LIVE_TMPVAR || kind == ZEND_LIVE_LOOP) {
-					ZVAL_COPY_VALUE(gc_buffer++, var);
+					zend_get_gc_buffer_add_zval(gc_buffer, var);
 				}
 			}
 		}
@@ -402,11 +337,12 @@ static HashTable *zend_generator_get_gc(zend_object *object, zval **table, int *
 	if (generator->node.children == 0) {
 		zend_generator *root = generator->node.ptr.root;
 		while (root != generator) {
-			ZVAL_OBJ(gc_buffer++, &root->std);
+			zend_get_gc_buffer_add_obj(gc_buffer, &root->std);
 			root = zend_generator_get_child(&root->node, generator);
 		}
 	}
 
+	zend_get_gc_buffer_use(gc_buffer, table, n);
 	if (EX_CALL_INFO() & ZEND_CALL_HAS_SYMBOL_TABLE) {
 		return execute_data->symbol_table;
 	} else {
diff --git a/Zend/zend_generators.h b/Zend/zend_generators.h
index abccf3a529..3a3d567508 100644
--- a/Zend/zend_generators.h
+++ b/Zend/zend_generators.h
@@ -89,9 +89,6 @@ struct _zend_generator {
 
 	/* ZEND_GENERATOR_* flags */
 	zend_uchar flags;
-
-	zval *gc_buffer;
-	uint32_t gc_buffer_size;
 };
 
 static const zend_uchar ZEND_GENERATOR_CURRENTLY_RUNNING = 0x1;
diff --git a/Zend/zend_globals.h b/Zend/zend_globals.h
index b7139fdfd6..298c22fe61 100644
--- a/Zend/zend_globals.h
+++ b/Zend/zend_globals.h
@@ -238,6 +238,8 @@ struct _zend_executor_globals {
 
 	zend_bool exception_ignore_args;
 
+	zend_get_gc_buffer get_gc_buffer;
+
 	void *reserved[ZEND_MAX_RESERVED_RESOURCES];
 };
 
diff --git a/ext/spl/spl_dllist.c b/ext/spl/spl_dllist.c
index 306aefa5d8..ef0e5b60ff 100644
--- a/ext/spl/spl_dllist.c
+++ b/ext/spl/spl_dllist.c
@@ -91,8 +91,6 @@ struct _spl_dllist_object {
 	zend_function         *fptr_offset_del;
 	zend_function         *fptr_count;
 	zend_class_entry      *ce_get_iterator;
-	zval                  *gc_data;
-	int                    gc_data_count;
 	zend_object            std;
 };
 
@@ -356,10 +354,6 @@ static void spl_dllist_object_free_storage(zend_object *object) /* {{{ */
 		zval_ptr_dtor(&tmp);
 	}
 
-	if (intern->gc_data != NULL) {
-		efree(intern->gc_data);
-	};
-
 	spl_ptr_llist_destroy(intern->llist);
 	SPL_LLIST_CHECK_DELREF(intern->traverse_pointer);
 }
@@ -534,21 +528,15 @@ static inline HashTable* spl_dllist_object_get_debug_info(zend_object *obj) /* {
 static HashTable *spl_dllist_object_get_gc(zend_object *obj, zval **gc_data, int *gc_data_count) /* {{{ */
 {
 	spl_dllist_object *intern = spl_dllist_from_obj(obj);
+	zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create();
 	spl_ptr_llist_element *current = intern->llist->head;
-	int i = 0;
-
-	if (intern->gc_data_count < intern->llist->count) {
-		intern->gc_data_count = intern->llist->count;
-		intern->gc_data = safe_erealloc(intern->gc_data, intern->gc_data_count, sizeof(zval), 0);
-	}
 
 	while (current) {
-		ZVAL_COPY_VALUE(&intern->gc_data[i++], &current->data);
+		zend_get_gc_buffer_add_zval(gc_buffer, &current->data);
 		current = current->next;
 	}
 
-	*gc_data = intern->gc_data;
-	*gc_data_count = i;
+	zend_get_gc_buffer_use(gc_buffer, gc_data, gc_data_count);
 	return zend_std_get_properties(obj);
 }
 /* }}} */
diff --git a/ext/spl/spl_observer.c b/ext/spl/spl_observer.c
index 09f27595d9..bcf3a39535 100644
--- a/ext/spl/spl_observer.c
+++ b/ext/spl/spl_observer.c
@@ -50,8 +50,6 @@ typedef struct _spl_SplObjectStorage { /* {{{ */
 	HashPosition      pos;
 	zend_long         flags;
 	zend_function    *fptr_get_hash;
-	zval             *gcdata;
-	size_t            gcdata_num;
 	zend_object       std;
 } spl_SplObjectStorage; /* }}} */
 
@@ -75,11 +73,6 @@ void spl_SplObjectStorage_free_storage(zend_object *object) /* {{{ */
 	zend_object_std_dtor(&intern->std);
 
 	zend_hash_destroy(&intern->storage);
-
-	if (intern->gcdata != NULL) {
-		efree(intern->gcdata);
-	}
-
 } /* }}} */
 
 static int spl_object_storage_get_hash(zend_hash_key *key, spl_SplObjectStorage *intern, zval *obj) {
@@ -285,23 +278,16 @@ static inline HashTable* spl_object_storage_debug_info(zend_object *obj) /* {{{
 /* overridden for garbage collection */
 static HashTable *spl_object_storage_get_gc(zend_object *obj, zval **table, int *n) /* {{{ */
 {
-	int i = 0;
 	spl_SplObjectStorage *intern = spl_object_storage_from_obj(obj);
 	spl_SplObjectStorageElement *element;
-
-	if (intern->storage.nNumOfElements * 2 > intern->gcdata_num) {
-		intern->gcdata_num = intern->storage.nNumOfElements * 2;
-		intern->gcdata = (zval*)erealloc(intern->gcdata, sizeof(zval) * intern->gcdata_num);
-	}
+	zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create();
 
 	ZEND_HASH_FOREACH_PTR(&intern->storage, element) {
-		ZVAL_COPY_VALUE(&intern->gcdata[i++], &element->obj);
-		ZVAL_COPY_VALUE(&intern->gcdata[i++], &element->inf);
+		zend_get_gc_buffer_add_zval(gc_buffer, &element->obj);
+		zend_get_gc_buffer_add_zval(gc_buffer, &element->inf);
 	} ZEND_HASH_FOREACH_END();
 
-	*table = intern->gcdata;
-	*n = i;
-
+	zend_get_gc_buffer_use(gc_buffer, table, n);
 	return zend_std_get_properties(obj);
 }
 /* }}} */