Skip to content

Use plain-array for static_variables #1707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nikic wants to merge 1 commit into from
Closed

Use plain-array for static_variables #1707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Zend/zend_builtin_functions.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1998,7 +1998,7 @@ ZEND_FUNCTION(create_function)

if (retval==SUCCESS) {
zend_op_array *func;
HashTable *static_variables;
zval *static_variables;

func = zend_hash_str_find_ptr(EG(function_table), LAMBDA_TEMP_FUNCNAME, sizeof(LAMBDA_TEMP_FUNCNAME)-1);
if (!func) {
Expand Down
47 changes: 34 additions & 13 deletions Zend/zend_closures.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,10 +424,20 @@ static HashTable *zend_closure_get_debug_info(zval *object, int *is_temp) /* {{{
ALLOC_HASHTABLE(debug_info);
zend_hash_init(debug_info, 8, NULL, ZVAL_PTR_DTOR, 0);

if (closure->func.type == ZEND_USER_FUNCTION && closure->func.op_array.static_variables) {
HashTable *static_variables = closure->func.op_array.static_variables;
ZVAL_ARR(&val, zend_array_dup(static_variables));
if (closure->func.type == ZEND_USER_FUNCTION && closure->func.op_array.last_static_var) {
int i;
zval *entry;

array_init(&val);
zend_hash_str_update(debug_info, "static", sizeof("static")-1, &val);
for (i = 0; i < closure->func.op_array.last_static_var; i++) {
entry = &closure->func.op_array.static_variables[i];
if (Z_TYPE_P(entry) == IS_REFERENCE && Z_REFCOUNT_P(entry) == 1) {
entry = Z_REFVAL_P(entry);
}
Z_TRY_ADDREF_P(entry);
zend_hash_update(Z_ARRVAL(val), closure->func.op_array.static_vars[i], entry);
}
}

if (Z_TYPE(closure->this_ptr) != IS_UNDEF) {
Expand Down Expand Up @@ -474,10 +484,15 @@ static HashTable *zend_closure_get_gc(zval *obj, zval **table, int *n) /* {{{ */
{
zend_closure *closure = (zend_closure *)Z_OBJ_P(obj);

*table = Z_TYPE(closure->this_ptr) != IS_NULL ? &closure->this_ptr : NULL;
*n = Z_TYPE(closure->this_ptr) != IS_NULL ? 1 : 0;
return (closure->func.type == ZEND_USER_FUNCTION) ?
closure->func.op_array.static_variables : NULL;
if (closure->func.type == ZEND_USER_FUNCTION) {
*n = closure->func.op_array.last_static_var + 1;
*table = closure->func.op_array.static_variables;
ZVAL_COPY_VALUE(*table + closure->func.op_array.last_static_var, &closure->this_ptr);
} else {
*n = Z_ISUNDEF(closure->this_ptr)? 0 : 1;
*table = Z_ISUNDEF(closure->this_ptr)? NULL : &closure->this_ptr;
}
return NULL;
}
/* }}} */

Expand Down Expand Up @@ -569,9 +584,15 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent
memcpy(&closure->func, func, sizeof(zend_op_array));
closure->func.common.prototype = (zend_function*)closure;
closure->func.common.fn_flags |= ZEND_ACC_CLOSURE;
if (closure->func.op_array.static_variables) {
closure->func.op_array.static_variables =
zend_array_dup(closure->func.op_array.static_variables);
closure->func.op_array.static_variables = (zval*)emalloc(sizeof(zval) * (func->op_array.last_static_var + 1));

if (func->op_array.static_variables) {
int i = func->op_array.last_static_var;
zval *static_variables = func->op_array.static_variables;
while (i > 0) {
i--;
ZVAL_COPY(&closure->func.op_array.static_variables[i], &static_variables[i]);
}
}
if (UNEXPECTED(!closure->func.op_array.run_time_cache)) {
closure->func.op_array.run_time_cache = func->op_array.run_time_cache = zend_arena_alloc(&CG(arena), func->op_array.cache_size);
Expand Down Expand Up @@ -626,11 +647,11 @@ ZEND_API void zend_create_fake_closure(zval *res, zend_function *func, zend_clas
}
/* }}} */

void zend_closure_bind_var(zval *closure_zv, zend_string *var_name, zval *var) /* {{{ */
void zend_closure_bind_var(zval *closure_zv, uint32_t num, zval *var) /* {{{ */
{
zend_closure *closure = (zend_closure *) Z_OBJ_P(closure_zv);
HashTable *static_variables = closure->func.op_array.static_variables;
zend_hash_update(static_variables, var_name, var);
zval *static_variables = closure->func.op_array.static_variables;
ZVAL_COPY_VALUE(&static_variables[num], var);
}
/* }}} */

Expand Down
2 changes: 1 addition & 1 deletion Zend/zend_closures.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
BEGIN_EXTERN_C()

void zend_register_closure_ce(void);
void zend_closure_bind_var(zval *closure_zv, zend_string *var_name, zval *var);
void zend_closure_bind_var(zval *closure_zv, uint32_t num, zval *var);

extern ZEND_API zend_class_entry *zend_ce_closure;

Expand Down
84 changes: 53 additions & 31 deletions Zend/zend_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ void zend_oparray_context_begin(zend_oparray_context *prev_context) /* {{{ */
*prev_context = CG(context);
CG(context).opcodes_size = INITIAL_OP_ARRAY_SIZE;
CG(context).vars_size = 0;
CG(context).static_vars_size = 0;
CG(context).literals_size = 0;
CG(context).backpatch_count = 0;
CG(context).in_finally = 0;
Expand Down Expand Up @@ -413,8 +414,7 @@ static int lookup_cv(zend_op_array *op_array, zend_string* name) /* {{{ */{
}
i++;
}
i = op_array->last_var;
op_array->last_var++;
i = op_array->last_var++;
if (op_array->last_var > CG(context).vars_size) {
CG(context).vars_size += 16; /* FIXME */
op_array->vars = erealloc(op_array->vars, CG(context).vars_size * sizeof(zend_string*));
Expand Down Expand Up @@ -1025,8 +1025,12 @@ ZEND_API void function_add_ref(zend_function *function) /* {{{ */
(*op_array->refcount)++;
}
if (op_array->static_variables) {
if (!(GC_FLAGS(op_array->static_variables) & IS_ARRAY_IMMUTABLE)) {
GC_REFCOUNT(op_array->static_variables)++;
zval *static_vars = op_array->static_variables;
int i = op_array->last_static_var;
op_array->static_variables = emalloc(op_array->last_static_var * (sizeof(zval)));
while (i > 0) {
i--;
ZVAL_COPY(&op_array->static_variables[i], &static_vars[i]);
}
}
op_array->run_time_cache = NULL;
Expand Down Expand Up @@ -3719,39 +3723,53 @@ void zend_compile_global_var(zend_ast *ast) /* {{{ */
}
/* }}} */

static uint32_t zend_lookup_static_var(zend_op_array *op_array, zend_string *name) /* {{{ */ {
int i = 0;
zend_ulong hash_value = zend_string_hash_val(name);

while (i < op_array->last_static_var) {
if (ZSTR_VAL(op_array->static_vars[i]) == ZSTR_VAL(name) ||
(ZSTR_H(op_array->static_vars[i]) == hash_value &&
ZSTR_LEN(op_array->static_vars[i]) == ZSTR_LEN(name) &&
memcmp(ZSTR_VAL(op_array->static_vars[i]), ZSTR_VAL(name), ZSTR_LEN(name)) == 0)) {
zend_string_release(name);
return i;
}
i++;
}
i = op_array->last_static_var++;
if (op_array->last_static_var > CG(context).static_vars_size) {
CG(context).static_vars_size += 8;
op_array->static_vars = erealloc(op_array->static_vars, CG(context).static_vars_size * sizeof(zend_string*));
op_array->static_variables = erealloc(op_array->static_variables, CG(context).static_vars_size * sizeof(zval));
}
op_array->static_vars[i] = zend_new_interned_string(name);
return i;
}
/* }}} */

static void zend_compile_static_var_common(zend_ast *var_ast, zval *value, zend_bool by_ref) /* {{{ */
{
znode var_node, result;
zend_op *opline;
uint32_t var_num;
zend_string *var_name = zend_ast_get_str(var_ast);

zend_compile_expr(&var_node, var_ast);

if (!CG(active_op_array)->static_variables) {
if (CG(active_op_array)->scope) {
CG(active_op_array)->scope->ce_flags |= ZEND_HAS_STATIC_IN_METHODS;
}
ALLOC_HASHTABLE(CG(active_op_array)->static_variables);
zend_hash_init(CG(active_op_array)->static_variables, 8, NULL, ZVAL_PTR_DTOR, 0);
if (zend_string_equals_literal(var_name, "this")) {
zend_error_noreturn(E_COMPILE_ERROR, "Cannot use $this as lexical variable");
}

if (GC_REFCOUNT(CG(active_op_array)->static_variables) > 1) {
if (!(GC_FLAGS(CG(active_op_array)->static_variables) & IS_ARRAY_IMMUTABLE)) {
GC_REFCOUNT(CG(active_op_array)->static_variables)--;
}
CG(active_op_array)->static_variables = zend_array_dup(CG(active_op_array)->static_variables);
if (zend_is_auto_global(var_name)) {
zend_error_noreturn(E_COMPILE_ERROR, "Cannot use auto-global as lexical variable");
}
zend_hash_update(CG(active_op_array)->static_variables, Z_STR(var_node.u.constant), value);

opline = zend_emit_op(&result, by_ref ? ZEND_FETCH_W : ZEND_FETCH_R, &var_node, NULL);
opline->extended_value = ZEND_FETCH_STATIC;
var_num = zend_lookup_static_var(CG(active_op_array), zend_string_copy(var_name));
ZVAL_COPY_VALUE(&CG(active_op_array)->static_variables[var_num], value);

if (by_ref) {
zend_ast *fetch_ast = zend_ast_create(ZEND_AST_VAR, var_ast);
zend_emit_assign_ref_znode(fetch_ast, &result);
} else {
zend_ast *fetch_ast = zend_ast_create(ZEND_AST_VAR, var_ast);
zend_emit_assign_znode(fetch_ast, &result);
}
opline = zend_emit_op(NULL, ZEND_FETCH_STATIC, NULL, NULL);
opline->op1_type = IS_CV;
opline->op1.num = lookup_cv(CG(active_op_array), zend_string_copy(var_name));
opline->op2.num = var_num;
opline->extended_value = by_ref;
}
/* }}} */

Expand Down Expand Up @@ -4896,7 +4914,7 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast) /* {{{ */
}
/* }}} */

static void zend_compile_closure_binding(znode *closure, zend_ast *uses_ast) /* {{{ */
static void zend_compile_closure_binding(zend_op_array *op_array, znode *closure, zend_ast *uses_ast) /* {{{ */
{
zend_ast_list *list = zend_ast_get_list(uses_ast);
uint32_t i;
Expand All @@ -4917,7 +4935,8 @@ static void zend_compile_closure_binding(znode *closure, zend_ast *uses_ast) /*

opline = zend_emit_op(NULL, ZEND_BIND_LEXICAL, closure, NULL);
opline->op2_type = IS_CV;
opline->op2.var = lookup_cv(CG(active_op_array), zend_string_copy(var_name));
opline->op2.num = lookup_cv(CG(active_op_array), zend_string_copy(var_name));
opline->result.var = op_array->last_static_var++;
opline->extended_value = by_ref;
}
}
Expand Down Expand Up @@ -5186,7 +5205,7 @@ void zend_compile_func_decl(znode *result, zend_ast *ast) /* {{{ */
} else {
zend_begin_func_decl(result, op_array, decl);
if (uses_ast) {
zend_compile_closure_binding(result, uses_ast);
zend_compile_closure_binding(op_array, result, uses_ast);
}
}

Expand All @@ -5209,8 +5228,11 @@ void zend_compile_func_decl(znode *result, zend_ast *ast) /* {{{ */

zend_compile_params(params_ast, return_type_ast);
if (uses_ast) {
/* the order must be the same as zend_compile_closure_binding */
op_array->last_static_var = 0;
zend_compile_closure_uses(uses_ast);
}

zend_compile_stmt(stmt_ast);

if (is_method) {
Expand Down
6 changes: 4 additions & 2 deletions Zend/zend_compile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ typedef struct _zend_live_range {
typedef struct _zend_oparray_context {
uint32_t opcodes_size;
int vars_size;
int static_vars_size;
int literals_size;
int backpatch_count;
int in_finally;
Expand Down Expand Up @@ -380,7 +381,9 @@ struct _zend_op_array {
zend_try_catch_element *try_catch_array;

/* static variables support */
HashTable *static_variables;
zend_string **static_vars;
zval *static_variables;
Copy link
Copy Markdown
Member Author

@nikic nikic Dec 31, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This naming is confusing... maybe static_var_names and static_vars?

Copy link
Copy Markdown
Member

@laruence laruence Dec 31, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, just meant to be similar with op_array->vars.

Copy link
Copy Markdown
Member

@dstogov dstogov Jan 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to keep an array of static variable names?

Copy link
Copy Markdown
Member

@bwoebi bwoebi Jan 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might use indirect addressing into vars array by using u2 inside static_variables?

int last_static_var;

zend_string *filename;
uint32_t line_start;
Expand Down Expand Up @@ -890,7 +893,6 @@ ZEND_API void zend_assert_valid_class_name(const zend_string *const_name);
/* global/local fetches */
#define ZEND_FETCH_GLOBAL 0x00000000
#define ZEND_FETCH_LOCAL 0x10000000
#define ZEND_FETCH_STATIC 0x20000000
#define ZEND_FETCH_GLOBAL_LOCK 0x40000000

#define ZEND_FETCH_TYPE_MASK 0x70000000
Expand Down
9 changes: 0 additions & 9 deletions Zend/zend_execute.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1537,15 +1537,6 @@ static zend_always_inline HashTable *zend_get_target_symbol_table(zend_execute_d
if (EXPECTED(fetch_type == ZEND_FETCH_GLOBAL_LOCK) ||
EXPECTED(fetch_type == ZEND_FETCH_GLOBAL)) {
ht = &EG(symbol_table);
} else if (EXPECTED(fetch_type == ZEND_FETCH_STATIC)) {
ZEND_ASSERT(EX(func)->op_array.static_variables != NULL);
ht = EX(func)->op_array.static_variables;
if (GC_REFCOUNT(ht) > 1) {
if (!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE)) {
GC_REFCOUNT(ht)--;
}
EX(func)->op_array.static_variables = ht = zend_array_dup(ht);
}
} else {
ZEND_ASSERT(fetch_type == ZEND_FETCH_LOCAL);
if (!EX(symbol_table)) {
Expand Down
15 changes: 12 additions & 3 deletions Zend/zend_inheritance.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,20 @@ static zend_function *zend_duplicate_function(zend_function *func, zend_class_en
/* reuse the same op_array structure */
return func;
}
if (!(GC_FLAGS(func->op_array.static_variables) & IS_ARRAY_IMMUTABLE)) {
GC_REFCOUNT(func->op_array.static_variables)++;
}
new_function = zend_arena_alloc(&CG(arena), sizeof(zend_op_array));
memcpy(new_function, func, sizeof(zend_op_array));
if (func->op_array.static_variables) {
int i = func->op_array.last_static_var;
if (func->op_array.fn_flags & ZEND_ACC_CLOSURE) {
new_function->op_array.static_variables = emalloc(sizeof(zval) * (func->op_array.last_static_var + 1));
} else {
new_function->op_array.static_variables = emalloc(sizeof(zval) * func->op_array.last_static_var);
}
while (i > 0) {
i--;
ZVAL_COPY(&new_function->op_array.static_variables[i], &func->op_array.static_variables[i]);
}
}
}
return new_function;
}
Expand Down
41 changes: 34 additions & 7 deletions Zend/zend_opcode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,10 @@ void init_op_array(zend_op_array *op_array, zend_uchar type, int initial_ops_siz
op_array->try_catch_array = NULL;
op_array->last_live_range = 0;

op_array->last_static_var = 0;
op_array->static_variables = NULL;
op_array->static_vars = NULL;

op_array->last_try_catch = 0;

op_array->this_var = -1;
Expand Down Expand Up @@ -135,9 +138,13 @@ ZEND_API void zend_function_dtor(zval *zv)

ZEND_API void zend_cleanup_op_array_data(zend_op_array *op_array)
{
if (op_array->static_variables &&
!(GC_FLAGS(op_array->static_variables) & IS_ARRAY_IMMUTABLE)) {
zend_hash_clean(op_array->static_variables);
if (op_array->static_variables) {
int i = op_array->last_static_var;
while (i > 0) {
i--;
zval_ptr_dtor(&op_array->static_variables[i]);
ZVAL_NULL(&op_array->static_variables[i]);
}
}
}

Expand Down Expand Up @@ -364,11 +371,13 @@ ZEND_API void destroy_op_array(zend_op_array *op_array)
zval *end;
uint32_t i;

if (op_array->static_variables &&
!(GC_FLAGS(op_array->static_variables) & IS_ARRAY_IMMUTABLE)) {
if (--GC_REFCOUNT(op_array->static_variables) == 0) {
zend_array_destroy(op_array->static_variables);
if (op_array->static_variables) {
i = op_array->last_static_var;
while (i > 0) {
i--;
zval_ptr_dtor(&op_array->static_variables[i]);
}
efree(op_array->static_variables);
}

if (op_array->run_time_cache && !op_array->function_name) {
Expand All @@ -382,6 +391,16 @@ ZEND_API void destroy_op_array(zend_op_array *op_array)

efree_size(op_array->refcount, sizeof(*(op_array->refcount)));

if (op_array->static_vars) {
i = op_array->last_static_var;
while (i > 0) {
i--;
zend_string_release(op_array->static_vars[i]);
}
efree(op_array->static_vars);
}


if (op_array->vars) {
i = op_array->last_var;
while (i > 0) {
Expand Down Expand Up @@ -627,6 +646,14 @@ ZEND_API int pass_two(zend_op_array *op_array)
op_array->literals = (zval*)erealloc(op_array->literals, sizeof(zval) * op_array->last_literal);
CG(context).literals_size = op_array->last_literal;
}

if (op_array->fn_flags & ZEND_ACC_CLOSURE) {
op_array->static_variables = (zval *)erealloc(op_array->static_variables, sizeof(zval) * (op_array->last_static_var + 1));
} else if (CG(context).static_vars_size != op_array->last_static_var) {
op_array->static_variables = (zval *)erealloc(op_array->static_variables, sizeof(zval) * op_array->last_static_var);
}
CG(context).static_vars_size = op_array->last_static_var;

opline = op_array->opcodes;
end = opline + op_array->last;
while (opline < end) {
Expand Down
Loading