Combine load_attr and store_attr type methods #1174
Conversation
This simplifies the API for objects and reduces code size (by around 400 bytes on Thumb2, and around 2k on x86). It reduced performance a little, measured as 1% decrease in Pystone score.
|
I think this is a very good patch, specially because of the RAM savings with the user defined objects. |
Not sure I understand this. Otherwise, the arguments above are very reasonable, though it's still a bit said that we'll use that 1% (and it won't be just 1% then) even for super-optimized viper code. I hope you'll recover opportunity to have 1 vs 2 methods in that meta-MicroPython you dream of ;-). |
I just meant that if you add a new object (eg pyb.I2S) then it'll cost 1 word less in ROM than it would have without this patch, because it has 1 less entry in mp_obj_type_t.
Yes, I'm not 100% happy to lose performance, hence why I made this a PR to get feedback. But I think we ought to do proper performance analysis to see where bytes are worth spending, rather than guessing.
That would certainly be the ideal situtation! It would even be possible now to make it configurable, but would be pretty ugly. |
I see, (kinda) singleton object you mean, because savings indeed are for types.
Yes. There're more #ifdef's coming all the time, and hopefully for more important things than trade-off between 1% performance and 0.5% of code space, so just go one way or another here. |
|
Will this patch have any impact if I'm writing I2S to make dynamic objects instead of static? |
|
No, it shouldn't have any impact either way. |
Except those do not run on most embedded platforms let alone pyboard? |
Yes, you have a point. uPy is a tradeoff amongst Python compatibility, code size, RAM usage and efficiency. It's a difficult balancing act. To confuse matters more, this patch actually improves pystone score on pyboard by 2%! Increases from 1717 to 1755. Because I don't understand this improvement I'm uncertain as to what this patch does under the hood. |
|
So, it decreases performance on x86 but improves it on thumb2? Interesting... |
|
It looks like @pfalcon and I are +/-0 with this, @danicampora is +1, @stinos is -1. Any more good arguments either way? |
|
Just rebased this PR and measured pystone performance on x64 and I'm getting no noticeable change with or without this patch. Same on pyboard. |
|
Well, I'm +0.75 on this, let's have this merged! |
|
Yes, +1 on merging it :-) |
|
No problem; for the record I wasn't really -1, more like 0: I didn't measure performance in an actual application myself and I'm not sure how well pystone score resembles what happens in an actual application? Just wanted to point out that while CPython/PyPy are nice, none of them nor any other I tried came even close to building on all kinds of 'minimal' platforms. |
Neither am I. But it seems to test quite a broad range of operations so is somewhat indicative of overall performance. Really we need a proper benchmark suite to make decisions about efficiency gains/loses.
uPy aims to be minimal and not inefficient. Low RAM usage is top priority, then low code size, then performance, although sometimes performance takes over low code size. If we didn't care about performance at all then it would be a different project. Running on a microcontroller or embedded environment also means you need to be efficient in CPU cycles. |
This simplifies the API for objects and reduces code size by combining the .load_attr and .store_attr methods in mp_obj_type_t.
Code size decreases:
In also saves RAM: each user-defined class is 1 word smaller. Furthermore, any further additions of new static objects will be 1 word smaller than previously.
But it does also hamper performance, although only very slightly. I measured about 1% decrease in pystone score on x86 (down from around 84k to around 83k).
My feeling is that this is a good patch to merge because it reduces code size by a non-trivial amount, and reduces RAM usage, making micropython more micro. If people want performance there's CPython and PyPy for that (and Pyston and Cython etc). If a port has bytes to spend on optimisations, then likely there are better things to spend those bytes on than separating load/store. Note that subscr load/store is already combined.