Re: com php-src: Optimize zend_mm_small_size_to_bin() It removes a cmov operation and has a much faster branch for small allocations (especially when
handling strings) (<= 64 bytes): Zend/zend_alloc.c Zend/zend_closures.c Zend/zend_execute.c

Hi Bob, that code was carefully designed to avoid branch miss-predictions. You saved few instructions but introduced branch instruction that often may take both paths. This mean, we may get branch miss-predictions, I-Cache misses, etc. Finally this may lead to slower execution. Did you measure the difference? Checks for op_array->cache_size was also omitted on purpose (to avoid extra checks). I'm tired to ask you showing your patches before commit. :( Thanks. Dmitry. On Sun, Sep 20, 2015 at 12:07 AM, Bob Weinand <bwoebi@php.net> wrote: > Commit: 08e253cdbc0a98e160cfd9c810ba213c7bcb3512 > Author: Bob Weinand <bobwei9@hotmail.com> Sat, 19 Sep 2015 > 23:07:43 +0200 > Parents: b8f56013b5501071a0fa5c74401afe861e5519f7 > Branches: PHP-7.0 > > Link: > > http://git.php.net/?p=php-src.git;a=commitdiff;h=08e253cdbc0a98e160cfd9c810ba213c7bcb3512 > > Log: > Optimize zend_mm_small_size_to_bin() > It removes a cmov operation and has a much faster branch for small > allocations (especially when handling strings) (<= 64 bytes) > > Changed paths: > M Zend/zend_alloc.c > M Zend/zend_closures.c > M Zend/zend_execute.c > > > Diff: > diff --git a/Zend/zend_alloc.c b/Zend/zend_alloc.c > index bf147f5..bd3fe0e 100644 > --- a/Zend/zend_alloc.c > +++ b/Zend/zend_alloc.c > @@ -1202,17 +1202,19 @@ static zend_always_inline int > zend_mm_small_size_to_bin(size_t size) > n = zend_mm_small_size_to_bit(size - 1); > return ((size-1) >> f1[n]) + f2[n]; > #else > - int t1, t2, t3; > - > - if (UNEXPECTED(size <= 8)) return 0; > - t1 = (int)(size - 1); > - t2 = zend_mm_small_size_to_bit(t1); > - t3 = t2 - 6; > - t3 = (t3 < 0) ? 0 : t3; > - t2 = t3 + 3; > - t1 = t1 >> t2; > - t3 = t3 << 2; > - return t1 + t3; > + unsigned int t1, t2; > + > + if (size <= 64) { > + /* we need to support size == 0 ... */ > + return (size - !!size) >> 3; > + } else { > + t1 = size - 1; > + t2 = zend_mm_small_size_to_bit(t1) - 3; > + t1 = t1 >> t2; > + t2 = t2 - 3; > + t2 = t2 << 2; > + return (int)(t1 + t2); > + } > #endif > } > > diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c > index 50aa7d4..251710e 100644 > --- a/Zend/zend_closures.c > +++ b/Zend/zend_closures.c > @@ -134,7 +134,7 @@ ZEND_METHOD(Closure, call) > fci_cache.function_handler = &my_function; > > /* Runtime cache relies on bound scope to be immutable, > hence we need a separate rt cache in case scope changed */ > - if (ZEND_USER_CODE(my_function.type) && > closure->func.common.scope != Z_OBJCE_P(newthis)) { > + if (ZEND_USER_CODE(my_function.type) && > closure->func.common.scope != Z_OBJCE_P(newthis) && > my_function.op_array.cache_size) { > my_function.op_array.run_time_cache = > emalloc(my_function.op_array.cache_size); > memset(my_function.op_array.run_time_cache, 0, > my_function.op_array.cache_size); > } > @@ -206,7 +206,7 @@ ZEND_METHOD(Closure, bind) > new_closure = (zend_closure *) Z_OBJ_P(return_value); > > /* Runtime cache relies on bound scope to be immutable, hence we > need a separate rt cache in case scope changed */ > - if (ZEND_USER_CODE(closure->func.type) && > (closure->func.common.scope != new_closure->func.common.scope || > (closure->func.op_array.fn_flags & ZEND_ACC_NO_RT_ARENA))) { > + if (ZEND_USER_CODE(closure->func.type) && > (closure->func.common.scope != new_closure->func.common.scope || > (closure->func.op_array.fn_flags & ZEND_ACC_NO_RT_ARENA)) && > new_closure->func.op_array.cache_size) { > new_closure->func.op_array.run_time_cache = > emalloc(new_closure->func.op_array.cache_size); > memset(new_closure->func.op_array.run_time_cache, 0, > new_closure->func.op_array.cache_size); > > @@ -543,7 +543,7 @@ ZEND_API void zend_create_closure(zval *res, > zend_function *func, zend_class_ent > > zend_hash_init(closure->func.op_array.static_variables, > zend_hash_num_elements(static_variables), NULL, ZVAL_PTR_DTOR, 0); > zend_hash_apply_with_arguments(static_variables, > zval_copy_static_var, 1, closure->func.op_array.static_variables); > } > - if (UNEXPECTED(!closure->func.op_array.run_time_cache)) { > + if (UNEXPECTED(!closure->func.op_array.run_time_cache) && > func->op_array.cache_size) { > closure->func.op_array.run_time_cache = > func->op_array.run_time_cache = zend_arena_alloc(&CG(arena), > func->op_array.cache_size); > memset(func->op_array.run_time_cache, 0, > func->op_array.cache_size); > } > diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c > index 67a6bc2..dcf2d4a 100644 > --- a/Zend/zend_execute.c > +++ b/Zend/zend_execute.c > @@ -2166,7 +2166,7 @@ static zend_always_inline void > i_init_func_execute_data(zend_execute_data *execu > GC_REFCOUNT(Z_OBJ(EX(This)))++; > } > > - if (UNEXPECTED(!op_array->run_time_cache)) { > + if (UNEXPECTED(!op_array->run_time_cache) && op_array->cache_size) > { > op_array->run_time_cache = zend_arena_alloc(&CG(arena), > op_array->cache_size); > memset(op_array->run_time_cache, 0, op_array->cache_size); > } > @@ -2193,7 +2193,7 @@ static zend_always_inline void > i_init_code_execute_data(zend_execute_data *execu > GC_REFCOUNT(Z_OBJ(EX(This)))++; > } > > - if (!op_array->run_time_cache) { > + if (!op_array->run_time_cache && op_array->cache_size) { > op_array->run_time_cache = emalloc(op_array->cache_size); > memset(op_array->run_time_cache, 0, op_array->cache_size); > } > > > -- > PHP CVS Mailing List (http://www.php.net/) > To unsubscribe, visit: http://www.php.net/unsub.php > >