Re: call_outs

Yes, I think its the only case. I guess that having many call_outs to only a few different functions is not so uncommon. Doing a lot of find_call_out/remove_call_out calls might be rare.

The reasonable thing would be to have the buckets in sorted order so that they could stop searching at the first hit. But read on..

...

Performance aside, it's not good that it can consume an unbounded amount of svalue stack either. At the very least there should be a check_stack() in there somewhere.

Hm, since bucket size is not known beforehand, it might be necessary to keep track of that.

Yes, so the check_stack call must be inside the loop. One can always make it run only every 100 iterations or so, but I don't know if it's worth the bother. It's pretty quick.

@@ -966,6 +966,7 @@ PIKECLASS Backend return c->pos; } }

• return -1;
     }
     /* Note: is_eq() may call Pike code (which we want),
  

  •   but Pike code may change the hash tables...
    

Not safe since an array can actually work as a callable:

call_out (({lambda() {werror ("oy!\n");}}), 0);

(1) Result: ({ /* 1 element */ ({ /* 1 element */ HilfeInput()->__lambda_65794_0_line_1 }) })

start backend oy!

@@ -981,6 +982,14 @@ PIKECLASS Backend if(CALL(c->pos) != c) Pike_fatal("Call_out->pos not correct!\n"); #endif

•   /* We dont need to check any others. These two
  

•    * will be equal.
  

•    */
  

•   if (is_identical(fun, ITEM(c->args))) {
  

•     pop_n_elems(Pike_sp - save_sp);
  

•     return c->pos;
  

•   }
  

• push_int(c->pos);
  push_svalue(ITEM(c->args));
  

  }

Interesting, but it wasn't what I had in mind. That one would consume just as much stack if is_eq actually is required to match the function, or if the function really doesn't match (although an unlucky hash collision would be required for that). What I meant was something like this (untested):

@@ -981,6 +981,9 @@ PIKECLASS Backend if(CALL(c->pos) != c) Pike_fatal("Call_out->pos not correct!\n"); #endif + if (Pike_sp > save_sp && + is_identical (Pike_sp - 1, ITEM (c->args))) + pop_n_elems (2); push_int(c->pos); push_svalue(ITEM(c->args)); }

Note that this replaces the previous entry rather than skipping the current one. That's to keep compat with the current behavior where it runs is_eq from the top of the stack, i.e. from the end of the bucket. That's also something your optimization misses.

But now when I look closer at the buckets, I notice that new call outs are added to the beginning, and this function searches from the end. So if the same function is added multiple times with call out times in reverse order then (find|remove)_call_out will find the last function in the queue rather than the first one. This test program verifies that:

int start_t; void co_func (int id) { werror ("id %d, t %d\n", id, time() - start_t); } int main() { werror ("Adding forwards\n"); for (int i = 0; i <= 2; i++) call_out (co_func, i, i); start_t = time(); remove_call_out (co_func); while (Pike.DefaultBackend.get_stats()->num_call_outs) Pike.DefaultBackend (0); werror ("Adding backwards\n"); for (int i = 2; i >= 0; i--) call_out (co_func, i, i); start_t = time(); remove_call_out (co_func); while (Pike.DefaultBackend.get_stats()->num_call_outs) Pike.DefaultBackend (0); }

I'd say that's a bug, or at least it doesn't agree with the documentation. :(

It also looks like a rehash shuffles the buckets a bit.