[wxPython] Patch to wxPoint_LIST_Helper and friends [long].

Hi Robin,

Here's the patch to helpers.cpp that I promised. The patch is against
wxPython-2.2.5. The patch should be applied by going to the src directory
and running "patch -Np1 < wxhelper.patch". Swig will need to be rerun since
I modified my_typemaps.i.

It allows wxPoint_LIST_Helper to accept any sequence of length-2 sequences
instead of just lists of tuples. It also plugs a couple of memory leaks and
catches nonnumeric objects. I discuss performance, API, memory leak
implications of the patch below.

Not too suprisingly this turned out to be somewhat more complicated than I
had originally thought. I had to mess with my_typemaps.i and helpers.h in
addition to helpers.cpp. Since I know very little about Swig, (although much
more than I knew yesterday), it would be good to look over my changes to
my_typemaps.i to make sure they make sense.

API:

I changed the API for wxPoint_LIST_Helper. It now takes a second argument in
which the sequence length is returned. This prevents evil code (see the
Pathological class in testwxpatch.py) from changing its mind about what
length it is. Without this change, there is a chance that Pathological or
its brethren could crash the interpreter. However, if you think c-code from
outside of wxPython is likely to be calling into wxPoint_LIST_Helper, then
probably it should be renamed wxPoint_SEQUENCE_Helper and a version that
maintains the old API should be included as wxPoint_LIST_Helper.

This probably wouldn't be a bad idea in any event, but I was reluctant to
mess with Swig much since I really don't know what I'm doing when it comes
to Swig.

I also changed both wxPoint_Helper and wxPoint_LIST_Helper to catch
nonnumerical values. In the past one could pass a tuple of strings where a
wxPoint was expected and it would "work". On my machine the strings were
interpreted as zeros, but I'm not sure that that is consistent across
different platforms. This now raises an error.

MEMORY LEAKS:

I fixed a memory leak in wxPoint_Helper: PySequence_GetItem returns a new
reference and o1 and o2 were not getting DECREFed. There are memory leaks in
most (all?) of the *_LIST_Helper functions as well: when an error is raised,
the temp array is not deallocated on the way out. I didn't want to touch too
much code at once, and these are pretty minor, so I left them alone (except
in wxPoint_LIST_Helper).

Setting testleaks to 1 in testwxpatch and then running it while watching the
memory used by the process (e.g. with top) will reveal the memory leak in
the old code. The memory use by the new code should be stable.

PERFORMANCE:

There was some question about how using the general sequence protocol would
affect speed. As shown by the data below, there is a signifigant impact: for
the best case (a list of tuples of ints) things slowed down by about 50%.
However the speed of plotting Numeric arrays is improved by over a factor of
four. The slowdown for the lists of tuples cases can be eliminated by adding
special cases to the code, but at the expense of complexity. I suspect that
most applications that are plotting large numbers of points use Numeric, so
I think that the first and fourth rows in the table below are most
important. (This is obviously a somewhat biased view). I personally don't
think the complexity that the fast version of the codes add is worth the
speed gain, so the patch is against the simpler new version. NEW-FAST is
included with this message as a separate file, but it has not been tested or
even scrutinized very much.

Table: Times for plotting 50,000 points using PlotLines (see
testwxpatch.py):

TEST NEW NEW-FAST OLD
array: 0.134961007614 0.134145037987
(0.634264634192)
[(x,y)]: 0.104322197374 0.0881805369118
0.0870843082012
[[x,y]]: 0.108765723018 0.103665577608
(0.613360413125)
intarray: 0.086473224949 0.0866711830694
(0.626794075783)
[(intx,inty)]: 0.0548769187145 0.0394333916741
0.0387480531744
[[intx,inty]]: 0.0564195728787 0.0531457489709
(0.462688805421)

Values in parentheses were computed by first using map(tuple, data) since
these wouldn't work directly with the old version.

SUMMARY:

Well that's about it. Let me know if you think that wxPoint_LIST_Helper
needs to be renamed to keep the API consistent. Or anything else that I
would need to do make the patch acceptable.

Regards,

-tim

P.S. I suspect this probably showed up on the mailing list already when I
wasn't paying attention, but I had to change the indent level of the wxc
target in the setup.py file to get wxc to compile (on nt). If it hasn't
shown up already, or you don't know what I'm talking about, let me know and
I'll supply some details.

P.P.S In order to get the generated swig files to compile (NT) I had to run
the attached swigfixer.py program over them. Anyone else run into this
issue?

swigfixer.py (359 Bytes)

fast_wxPoint_LIST_Helper.cpp (2.46 KB)

wxhelper.patch (6.42 KB)

testwxpatch.py (4.16 KB)

Here's the patch to helpers.cpp that I promised. The patch is against
wxPython-2.2.5.

Thanks. I'll take a look sometime in the next few days.

P.S. I suspect this probably showed up on the mailing list already when I
wasn't paying attention, but I had to change the indent level of the wxc
target in the setup.py file to get wxc to compile (on nt). If it hasn't
shown up already, or you don't know what I'm talking about, let me know

and

I'll supply some details.

Yep, this has been fixed in CVS already.

P.P.S In order to get the generated swig files to compile (NT) I had to

run

the attached swigfixer.py program over them. Anyone else run into this
issue?

I have a patch for SWIG in CVS for this, but for some reason I was excluding
it from the .tar.gz. It'll be there next time.

Thanks again!

···

--
Robin Dunn
Software Craftsman
robin@AllDunn.com Java give you jitters?
http://wxPython.org Relax with wxPython!

_______________________________________________
wxPython-users mailing list
wxPython-users@lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/wxpython-users

Hi Robin,

Don't hurry to look at this too quickly. Due to Gordon's worry about speed
I'm putting some work in on the 'fast' version. Having been prodded into
working on it some more, I think I can simplify enough to make me happy and
then there will be no performance impact(*) on code using lists of tuples.

-tim

>
> Here's the patch to helpers.cpp that I promised. The patch is against
> wxPython-2.2.5.

Thanks. I'll take a look sometime in the next few days.

>
> P.S. I suspect this probably showed up on the mailing list already when

I

> wasn't paying attention, but I had to change the indent level of the wxc
> target in the setup.py file to get wxc to compile (on nt). If it hasn't
> shown up already, or you don't know what I'm talking about, let me know
and
> I'll supply some details.

Yep, this has been fixed in CVS already.

>
> P.P.S In order to get the generated swig files to compile (NT) I had to
run
> the attached swigfixer.py program over them. Anyone else run into this
> issue?
>

I have a patch for SWIG in CVS for this, but for some reason I was

excluding

···

it from the .tar.gz. It'll be there next time.

Thanks again!

--
Robin Dunn
Software Craftsman
robin@AllDunn.com Java give you jitters?
http://wxPython.org Relax with wxPython!

_______________________________________________
wxPython-users mailing list
wxPython-users@lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/wxpython-users

_______________________________________________
wxPython-users mailing list
wxPython-users@lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/wxpython-users

Tim Hochberg wrote:

I also changed both wxPoint_Helper and wxPoint_LIST_Helper to catch
nonnumerical values. In the past one could pass a tuple of strings where a
wxPoint was expected and it would "work". On my machine the strings were
interpreted as zeros, but I'm not sure that that is consistent across
different platforms. This now raises an error.

What impact does this have on perfomance? if you have to check every
tuple to make sure it is a tuple of numbers than you could be spending a
whole lot of time doing type checking. How have you made this efficient?
I ask because I find myself writing extensions that don't have good
error checking to avoid the slow down. If I was expecting a list of
two-tuples, for instance, I would check the first item in the list to
make sure it is correct, but not the rest of the list, as that would be
kind of slow. This, of course, is the problem with dynamic languages,
but with NumPy arrays, at least you have a sequence that is known to be
homogeneous. Given this, I'm surprised that your new code is slower with
NumPy arrays that with lists of tuples. Why do you think this is?

-CHB

···

--
Christopher Barker,
Ph.D.
cbarker@jps.net --- --- ---
http://www.jps.net/cbarker -----@@ -----@@ -----@@
                                   ------@@@ ------@@@ ------@@@
Water Resources Engineering ------ @ ------ @ ------ @
Coastal and Fluvial Hydrodynamics ------- --------- --------
------------------------------------------------------------------------
------------------------------------------------------------------------

_______________________________________________
wxPython-users mailing list
wxPython-users@lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/wxpython-users

Chris Barker writes:

I'm surprised that your new code is slower with NumPy
arrays that with lists of tuples.

Hi all,
First off I am very new to wxPython so I may be missing
something obvious ... but ..

Why don't we borrow the numpy helpers from pyOpenGL
Then you can pass an unlimited number of points at 'C' speed

Cheers

Norman Vine

static PyObject *py_gl_Vertex2f(PyObject * self, PyObject * args)
{
    GLfloat arg1, arg2;
#ifdef NUMERIC
    GLfloat *vert;
    PyObject *op;
    int n;
#endif
    if (PyArg_ParseTuple(args, "ff", &arg1, &arg2))
  glVertex2f(arg1, arg2);
#ifdef NUMERIC
    else {
  PyErr_Clear();
  TRY(PyArg_ParseTuple(args, "O", &op));
  TRY(PyArray_AsFloatArray(&op, &vert, &n));
  if (n < 2) {
      PyErr_SetString(py_gl_Error, "need element with at least 2 items");
      PyArray_ClearMemory(op, vert);
      return NULL;
  }
!!!!!!! THIS IS PASSING 'C' POINTER to an array of floats !!!!!!!
  glVertex2fv(vert);
  PyArray_ClearMemory(op, vert);
    }
#endif
    Py_INCREF(Py_None);
    return Py_None;
}

···

_______________________________________________
wxPython-users mailing list
wxPython-users@lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/wxpython-users

Tim Hochberg wrote:
> I also changed both wxPoint_Helper and wxPoint_LIST_Helper to catch
> nonnumerical values. In the past one could pass a tuple of strings where

a

> wxPoint was expected and it would "work". On my machine the strings were
> interpreted as zeros, but I'm not sure that that is consistent across
> different platforms. This now raises an error.

What impact does this have on perfomance? if you have to check every
tuple to make sure it is a tuple of numbers than you could be spending a
whole lot of time doing type checking.

Actually, since I'm special casing the heck out of everything now, both of
these checks speed things up for the common case. When I detect a tuple or
list I use PyList_Fast_GetItem which is signifigantly faster than
PySequence_GetItem (and saves me a pair of Py_DECREFs too). Similarly I
check if an object is a PyFloat or a PyInt and use fast methods to get at
the data. I'm probably taking a bit of a speed hit in the uncommon cases,
but since they didn't work at all before I'm not going to sweat it. The case
I really care about (arrays) is 10x faster this way than the way I was
forced to do it before.

How have you made this efficient?

1) Special casing.

2) It seems the XXX_Check() methods are fast -- I use them if possible to
avoid stuff like:.

PyObject* o = PySequence_GetItem(source, i);
if (o == Null) {....}
....
Py_DECREF(o);

If you know source is tuple here, you can use PyTuple_GetItem or
PySequence_Fast_GET_ITEM which are much faster and don't create a new
reference that needs DECREFed later.

Anyway, look at the latest patch and steal what I did there. If you have
questions or find a bug, be sure to let me know.

Make sure to look at the C API docs carefully since what creates new
references and what borrows references can be confusing. PySequence_GetItem
creates a new reference, while PyTuple_GetItem does not. That'll trip you
up!

I ask because I find myself writing extensions that don't have good
error checking to avoid the slow down. If I was expecting a list of
two-tuples, for instance, I would check the first item in the list to
make sure it is correct, but not the rest of the list, as that would be
kind of slow.

Although you'd probably notice it, I think PyTuple_Check and PyTuple_Size
are fairly fast. I just checked and removing all the checks for tuples of
length two gained me maybe a few percent in speed. Maybe not even that.

This, of course, is the problem with dynamic languages,
but with NumPy arrays, at least you have a sequence that is known to be
homogeneous. Given this, I'm surprised that your new code is slower with
NumPy arrays that with lists of tuples. Why do you think this is?

Because I'm not using the NumPy API to get at the arrays. Instead I'm using
the general sequence API. This is still a lot faster than converting arrays
to lists of tuples, but slower than it could be. I suspect that were I to
use the NumPy API directly the conversion would be essentially
instantaneous. The disadvantage would be that wxPython would have to be
compiled against the NumPy libraries to get at the API. This also means that
the NumPy sections would have to surrounded by ifdefs. If there comes a time
when I need the speed, I'll do as Norman suggest and steal some code from
PyOpenGl and use the NumPy API.

-tim

···

_______________________________________________
wxPython-users mailing list
wxPython-users@lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/wxpython-users

Tim Hochberg wrote:

Actually, since I'm special casing the heck out of everything now, both of
these checks speed things up for the common case. When I detect a tuple or
list I use PyList_Fast_GetItem which is signifigantly faster than

I couldn't find PyLIst_Fast_GetItem in the PYthon C API reference docs.
where did yo find it?

2) It seems the XXX_Check() methods are fast -- I use them if possible to
avoid stuff like:.

PyObject* o = PySequence_GetItem(source, i);
if (o == Null) {....}
....
Py_DECREF(o);

I get it now.

Anyway, look at the latest patch and steal what I did there. If you have
questions or find a bug, be sure to let me know.

Where is this patch? I havn't seen a version newer than the
fast_wxPoint_LIST_Helper.cpp file that you enclosed with the first
batch. I understand you have added to this. Could you send that along?

Make sure to look at the C API docs carefully since what creates new
references and what borrows references can be confusing. PySequence_GetItem
creates a new reference, while PyTuple_GetItem does not. That'll trip you
up!

I know this can get messy!

Although you'd probably notice it, I think PyTuple_Check and PyTuple_Size
are fairly fast. I just checked and removing all the checks for tuples of
length two gained me maybe a few percent in speed. Maybe not even that.

Speed has to reletive to something. I guess I've been trying to get
speed close to that of native C arrays. The only way I can do that is to
use NumPy arrays, whoich brings us to:

Because I'm not using the NumPy API to get at the arrays. Instead I'm using
the general sequence API. This is still a lot faster than converting arrays
to lists of tuples, but slower than it could be. I suspect that were I to
use the NumPy API directly the conversion would be essentially
instantaneous.

OK I see it now. (and I took a look at your first draft of ...fast... )
I was, in fact, imagining the speed compared to using the NumPy API.

The disadvantage would be that wxPython would have to be
compiled against the NumPy libraries to get at the API. This also means that
the NumPy sections would have to surrounded by ifdefs. If there comes a time
when I need the speed, I'll do as Norman suggest and steal some code from
PyOpenGl and use the NumPy API.

HMMM. This has really been killing a lot of efforts to get good
performance out of Python. The only way to get good performance with a
lot of numbers on Python is to Use Numeric (it also gives you real
multi-dimensional slicing) That's why I was surprised that Gordon isn't
using it. The NumPy API is pretty easy to use, I say we use it! It also
appears that it (or at least something similar) will be included as a
standard part of Python in the future. (PEP # 209)

Robin, what do you think about putting some NumPy support into wxPython?
I think it could be
pretty darn handy and fast.

Maybe it makes sense to use the PyOpenGl code, but it would be pretty
easy to do from scratch as well. In fact, if going from a NumPy array to
a PointList would be essentially instantanious, then the easy way to do
it would be to use PyArray_ContiguousFromObject and PyArray_Cast (in the
NumPy library) Which would take care of all the other possible inputs
for us (I think it's pretty well optimized). That would actaully make
for simpler code.

If you send me your latest fast_wxPoint_LIST_Helper.cpp file, I'll try
to add this, and we can have the best of all worlds!

-Chris

···

--
Christopher Barker,
Ph.D.
cbarker@jps.net --- --- ---
http://www.jps.net/cbarker -----@@ -----@@ -----@@
                                   ------@@@ ------@@@ ------@@@
Water Resources Engineering ------ @ ------ @ ------ @
Coastal and Fluvial Hydrodynamics ------- --------- --------
------------------------------------------------------------------------
------------------------------------------------------------------------

_______________________________________________
wxPython-users mailing list
wxPython-users@lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/wxpython-users

Robin, what do you think about putting some NumPy support into wxPython?
I think it could be
pretty darn handy and fast.

There are already too many external dependencies for wxPython, I don't want
to add another. If you want to create a version with NumPy #ifdef's and
supporting code for setup.py to optionally turn it on then I'll take that,
but I don't want to require NumPy by default.

···

--
Robin Dunn
Software Craftsman
robin@AllDunn.com Java give you jitters?
http://wxPython.org Relax with wxPython!

_______________________________________________
wxPython-users mailing list
wxPython-users@lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/wxpython-users

Hi Chris,

[SNIP]

I couldn't find PyLIst_Fast_GetItem in the PYthon C API reference docs.
where did yo find it?

My mistake, it's PyList_Fast_GET_ITEM and it's under section 6.3 (sequence
items) of the C API docs. Way down at the end.

[SNIP]

Where is this patch? I havn't seen a version newer than the
fast_wxPoint_LIST_Helper.cpp file that you enclosed with the first
batch. I understand you have added to this. Could you send that along?

I sent out a new patch earlier (wxhelper-2.patch). Did you get that. It's a
unified diff, not a source file, but if you open it up you can see what I
did.

OK I see it now. (and I took a look at your first draft of ...fast... )
I was, in fact, imagining the speed compared to using the NumPy API.

No, not even close. However there is a limit set by the drawing routines
themselves. I did an experiment yesterday where all I did was allocate the
array and measured the draw times. These times were about 0.028 seconds for
50,000 points worth of lines using DrawLines. This is as compared to 0.035
seconds for tuples and 0.077 seconds for arrays (with the patch). So, you
shouldn't be able to speed up DrawLines by more than about a factor of 2.5
even using Numeric directly.

The NumPy API is pretty easy to use, I say we use it! It also
appears that it (or at least something similar) will be included as a
standard part of Python in the future. (PEP # 209)

I hope so. Maybe there will be some news after the conference.

[SNIP]

Maybe it makes sense to use the PyOpenGl code, but it would be pretty
easy to do from scratch as well. In fact, if going from a NumPy array to
a PointList would be essentially instantanious, then the easy way to do
it would be to use PyArray_ContiguousFromObject and PyArray_Cast (in the
NumPy library) Which would take care of all the other possible inputs
for us (I think it's pretty well optimized). That would actaully make
for simpler code.

It's worth a try, but I suspect you'll kill the performance of the list of
tuples case by doing this. I think you'll need two branches one for lists of
tuples and one for arrays.

If you send me your latest fast_wxPoint_LIST_Helper.cpp file, I'll try
to add this, and we can have the best of all worlds!

I'll send you the file (it's actually a chunk out of helpers.cpp), but Robin
doesn't seem to keen on adding dependances on NumPy and I can't really blame
him. If your not in a huge hurry for this, this seems like an idea it might
be better to wait on till an array object ends up in the core.

-tim

wxPoint_LIST_helper.cpp (2.4 KB)

···

From: "Chris Barker" <cbarker@jps.net>

Chris Barker writes:

Maybe it makes sense to use the PyOpenGl code, but it would be pretty
easy to do from scratch as well.

I would look at pyOpenGL / arraystuff.c

There are some potentially useful 'cheap' utility
functions that are written for both when Numpy
compiled in and not that I would suggest 'borrowing'.

see
PyArray_As%%%Array( PyObject **op, %%% **pitems, int *pn)
%%% == 'type'

Norman Vine

···

_______________________________________________
wxPython-users mailing list
wxPython-users@lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/wxpython-users

[Chris]

> Robin, what do you think about putting some NumPy support into wxPython?
> I think it could be
> pretty darn handy and fast.

[Robin]

There are already too many external dependencies for wxPython, I don't

want

to add another. If you want to create a version with NumPy #ifdef's and
supporting code for setup.py to optionally turn it on then I'll take that,
but I don't want to require NumPy by default.

There's another approach, which I hesitate to mention since I'm not sure
that I approve of it, but what the heck. Once can use the Object protocol to
ask an object for its size attribute and also call the object's tostring
method to get a string. If the object is not an array this will fail, at
which point you fall back on the standard sequence extraction stuff. The
tostring method, unlike the tolist method, is fast and it returns a copy of
the (flattened, contiguousified) underlying array data. Once one has the
string, it can be copied into an array of wxPoints very quickly. This has a
lot of overhead for short arrays, but for long arrays it's fast.

I tried this, and the results are impressive: .031 seconds to draw 50,000
points as opposed .035 seconds for lists of tuples and .028 seconds just to
draw the points. So, of that .031 seconds, the extraction is taking about
.003 seconds, not bad when compared with the patch where the extraction is
taking .044 seconds (out of .075 seconds total to draw the points). However,
the net gain is only about a factor of two since the plot time becomes
dominated by wxWindows' DrawLines.

For me, the extra complexity is not worth it; the current patch is fast
enough. I've already gained a factor of ten, and I don't need that last
factor of two. Also, using ifdefs and Numeric code is much cleaner and has
less overhead if you really need the speed. The only advantage of this
approach is that it would work with Numeric "out of the box" without needing
to be recompiled. The current patch already does that (although it's
slower), and like I said, I don't think the extra complexity is worth it.

However, if anyone wants to play with the code, let me know and I'll send
you a copy.

-tim

···

_______________________________________________
wxPython-users mailing list
wxPython-users@lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/wxpython-users