[issue1530559] struct.pack raises TypeError where it used to convert

[Meador Inge]

Meador Inge <meadori at gmail.com> added the comment:

> (2) For 2.x, I'm a bit uncomfortable with introducing the extra Python layer 
> on top of the C layer.  Partly I'm worried about accidentally breaking 
> something (it's not 100% clear to me whether there might be hidden 
> side-effects to such a change),

I understand.  I am worried about that as well. The main area that 
concerns me is the interface for the 'Struct' class.  I did my best to 
match the methods and functions from the C implementation.  I need to 
look into this in more detail, though.  One quirk I currently know 
about is that the following methods:

   '__delattr__', '__getattribute__', '__setattr__', '__new__'

show up in 'help' for the C implementation, but not the Python one.  
Although, the implementations do exist in both places. 

> but I also notice that this seems to have a significant impact on performance.
> In fact, I seem to recall that the previously existing Python component of 
> the struct module was absorbed into Modules/_struct.c precisely for 
> performance reasons.
>
> A quick, unscientific benchmark:  the time taken to run test_struct with this
> patch (excluding the changes to test_struct itself) on my machine (OS X 10.6,
> 64-bit non-framework non-debug build of Python) is around 1.52--1.53 seconds; > without the patch it's around 1.02--1.03 seconds.

Agreed that this is not a really scientific benchmark.  I did 
experiment with this idea a little further though on my machine (OS X 
10.5 32-bit):

==============================================
| Configuration                    | Seconds |
==============================================
| Original                         | 1.26    |
----------------------------------------------
| __index__ patch v1               | 1.88    |
----------------------------------------------
| Hoisted imports out of functions | 1.53    |
----------------------------------------------
| Hoisted imports and no __index__ | 1.34    |
----------------------------------------------

So with this simple experiment pulling the 'imports' out of the function
wrappers made quite a difference.  And, of course, removing the 
'__index__' transform brought the times down even more.  So, the 
wrapper overhead does not look to be terrible for this simple test.

However, as you alluded to, we should find a better benchmark.  Any 
ideas on a good one?
 
> (4) For 2.x, perhaps we don't need the extra __index__ functionality anyway, 
> now that the previous use of __int__ has been restored.  That would give us 
> a bit more time to think about this for 3.x.

Agreed.  Thanks for taking the time to look at the patch anyway!

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue1530559>
_______________________________________