[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [pygame] State of the math branch

Hi,

first of all thanks for the feedback.
second I just merged the branch with the trunk. I hope I didn't mess anything
up (my merging experience is *very* limited).
I also implemented the generic math function René suggested.


René Dudfield wrote:

On Mon, Nov 16, 2009 at 6:10 PM, Casey Duncan <casey@xxxxxxxxxxx> wrote:

To start with, it is great to see follow-through on this, it will be a
great, long overdue, addition to the pygame api.

One thing I notice is that the main vector data structure contains a pointer
to the actual coordinates. Other structures just contain the coordinates in
an inline array up to the max dimension. IMO the main vector data structure
should have the coordinates inline as well. This will cut in half the number
of heap allocations to create a vector object. I suspect that it may be
common for vector objects to be created as intermediate results in
calculations and immediately discarded, and eliminating the extra memory
management would be a helpful optimization, and simplify the code (less
memory management is always a good thing). I think the only use-case for
having the pointer would be for proxying an array of vector coordinates that
you could operate on en masse (ala numpy).



Having the vector be a proxy for other data is big use case though.

How about...   There could be a double[4] inline, and have the pointer
point to that data for 2,3,4 sized vectors.  If it's bigger than this,
it can be (py)malloc'd, and it can also refer to data via the buffer
protocol.  The pointer would still be where the data is, but it could
be refer to the inline data if needed.



The reason I allocated the memory dynamically is that I didn't want to waste the
2*sizeof(double) on Vector2 instances. but maybe we don't have to be that cheep
in this day and age. especially if we can gain some speed. Another consideration
is that this way we can *maybe* reuse vectors as views on rows or columns of
matrices. I'm not convinced that thats a good idea or feasible but it is a
thought to keep in mind.




A further optimization to consider would be an instance pool for vectors. If
I'm right, and vectors are created and destroyed a lot in certain
applications, having a pool of unused instances lying around could be a real
win. This is how Python optimizes tuple object creation.



+1

pygame could use this technique all over.  I think using pymalloc can
give us this automatically... or close to automatically.

There's some notes here about performance here of pymalloc Vs free
lists(python terminology for memory pools):
    http://bugs.python.org/issue2039

Probably not as fast as a specific memory pool... but maybe pymalloc
would come close.



Yes. but before I totally agree I would like to see some benchmarks.
But this is an implementation detail. Until now I didn't focus on performance.
My main concern was a nice API. But that doesn't mean you shouldn't keep
posting suggestions on how to improve all aspects!




I didn't check thoroughly for this, but you need to be careful with code
like this (in vector_elementwiseproxy_pow):

   for (i = 0; i < dim; i++) {
       base = PyFloat_FromDouble(bases[i]);
       expo = PyFloat_FromDouble(expos[i]);
       result = PyNumber_Power(base, expo, Py_None);
       if (!result)
           return NULL;
       ret->coords[i] = PyFloat_AsDouble(result);
       Py_DECREF(result);
       Py_DECREF(expo);
       Py_DECREF(base);
   }

For one, PyFloat_FromDouble can return NULL, and I'm not sure what
PyNumber_Power will do when passed a NULL arg. Assuming it doesn't crash and
returns NULL, you may be leaking references to base or expo. It's probably
safer to assume PyNumber_Power will crash however, which is very bad since
it will mask the memory problem with a segfault. All of this conversion back
and forth from Python floats seems unnecessary, however.



Can't you just use
the libc pow() function directly instead, saving all the PyFloat business?



good idea


Well, I looked at the pow implementation of float objects in python-trunk and
they were catching some corner-cases and platform quirks. I wanted to take
advantage of their work. of course we could decide that performance is
paramount and people will have to live with whatever quirks their libc might
have. Note that the python math module did take this route which simply
interfaces the libc functions.
On the bad error handling and potential memory leaks you are 100% right!


I'm also curious what the use-case is for enabling and disabling swizzling?
I know I've seen other libraries that have this switch (Euclid?), but why
not just leave it on all of the time? Too error prone?


Well I for one had never heard of swizzling before I looked at euclid. And I
imagine I would have been deeply confused if vec.xyz = (1,2,3) wouldn't add
a new attribute. That's why I disabled swizzling per default. More or less
personal preference.


again, thanks for the feedback!

yours
//Lorenz