hpyproject / hpy

HPy: a better API for Python
https://hpyproject.org
MIT License
1.02k stars 52 forks source link

HPy_ErrorResult_IndexError (for better performance low level python code) #270

Open paugier opened 2 years ago

paugier commented 2 years ago

In https://github.com/hpyproject/hpy/issues/263, @steve-s discovered that the line HPyErr_SetString(ctx, ctx->h_IndexError, "index out of range"); leads to a performance issue for code looping over a array defined with HPy (in practice a piconumpy array). If I understand well, he proposed a possible solution involving an extension of HPy API:

I think that even for Pypy it would be useful if it knew that it actually does not have to create the exception. If we agree this is really important use case, we could, for example, provide some token that, if returned, would indicate that the HPy function wishes to exit with an exception of type IndexError, so the code would be:

HPyDef_SLOT(Array_item, Array_item_impl, HPy_sq_item)
HPy Array_item_impl(HPyContext *ctx, HPy h_arr, HPy_ssize_t index) {  
  ArrayObject *arr = ArrayObject_AsStruct(ctx, h_arr);
  if (index < 0 || index >= arr->size) {
    return HPy_ErrorResult_IndexError;
  }
  HPy item = HPyFloat_FromDouble(ctx, arr->data[index]);
  return item;
};

It seems that this issue with HPyErr_SetString(ctx, ctx->h_IndexError, "index out of range"); also applies to tp_iter (https://github.com/hpyproject/hpy/issues/263#issuecomment-988111355).

Since https://github.com/hpyproject/hpy/issues/263 is too broad, I create this issue to discuss this specific proposition.

This addition could drastically improved the performance of low level Python code looping over an array defined with HPy (more than an order of magnitude speedup given the results described in https://github.com/hpyproject/hpy/issues/263).

steve-s commented 2 years ago

@hodgestar brought to my attention that this is already the contract of tp_iternext (https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_iternext):

When the iterator is exhausted, it must return NULL; a StopIteration exception may or may not be set.

This should be also the contract in HPy. I would keep this issue open, because I think the next thigh we can try is to update piconumpy HPy to use tp_iternext in this way and see how it changes the benchmark results.

paugier commented 2 years ago

Just a note: I guess tp_iternext will be used for loops like for value in arr: (microbench sum_loop) but not in the case of direct indexing of the array in the loop (microbench sum_loop_index, which is a very common pattern).

steve-s commented 2 years ago

As long as the exception is never thrown at runtime, i.e., you check the terminating condition yourself preemptively without raising and catching the exception, it should be fine.