Repository navigation
Conversation
|
This approached #2622 (comment) from another side. This code will be possible to reuse functionality described there. In the meantime, this patch adds 131 byte on x86. |
|
(This issue popped up when I tried to implement resource_stream() in Python, using frozen bytecode). |
dpgeorge
left a comment
dpgeorge
left a comment
There was a problem hiding this comment.
Code otherwise looks ok. Just a question of whether it's worth the code size/complexity increase for this minor (?) feature.
|
|
||
| data = b"1234" * 256 | ||
|
|
||
| before = micropython.mem_total() |
There was a problem hiding this comment.
mem_total() is not available on a lot of ports so there will need to be a feature check for it at the start.
| @@ -33,6 +33,8 @@ typedef struct _mp_obj_stringio_t { | |||
| vstr_t *vstr; | |||
| // StringIO has single pointer used for both reading and writing | |||
| mp_uint_t pos; | |||
| // Underlying object buffered by this StringIO | |||
| mp_obj_t ref_obj; | |||
There was a problem hiding this comment.
Is the sole purpose of this to prevent the underlying object from being GC'd? If so then probably it's not needed for str/bytes because they always have their data stored in a separate GC block, and StringIO here holds a pointer to that in the vstr.
There was a problem hiding this comment.
Is the sole purpose of this to prevent the underlying object from being GC'd?
Yes, it's done with that in mind, but also to cover what's described in #2622 (comment) - getvalue() would cast BytesIO's internal vstr to bytes, and store it there, subsequent calls will return it, and if code starts to write to it again, it will be COWed by the code added by this patch transparently. That of course means there's extra pointer to keep that string alive. But to get around that, there would be needed a dedicated method like .stealvalue() as discussed by the link above. Otherwise, we can just go "heuristic" method of clearing such cached string on .close/.truncate.
First of all, the overall context: with 1.9 release, we've spent too much time on same old backlogged issues, so it would be nice to work on something "fresh" (which is of course still old ideas discussed long ago). Specific context: Getting uasyncio be usable on esp8266. Given all effort put into it, I skip "bottom up" approach and go straight to running https://github.com/pfalcon/notes-pico . Then first step is how to deploy it comfortably. Ideally, it should be Back to overall picture: to get uasyncio running on low-heap systems, we really need to fill the gaps in buffer management, because constant coroutine allocation traffic doesn't leave room for allocation of large buffers, instead reference, iterable, and in-place approaches should be used. So yep, we're coming to the crux of that "we don't have code space for that feature" - we don't have, because we need much more space to finish features we already started. So, this is not a "minor" feature, it's one of the many features which need to be implemented to allow efficient memory management. One problem here is that maybe in the end, it may become clear that 5-6 features would cover it all. But it's hard to tell beforehand, so my need to implement dozen+ shortcuts for and between different types. And IMHO it's better to do that, even if some features will be removed then (e.g. this patch would allow to remove (need to discuss first) the uio.resource_stream patch above). Because otherwise it includes many moving parts, and it's just hard to approach it. |
…e policy. Don't create copy of immutable object's contents until .write() is called on BytesIO.
|
Merged. |
I'd say those features are already finished. It's just that one finds that they don't work as expected on really minimal targets and so they all need to be revisited. Is it really worth to hack up the code so much just to try and support a target which may not work in the end anyway? I don't want uPy to be twisted to fit esp8266, which in a few years will probably become obsolete. |
Then we again suddenly have different view at things, after working on them together for years, and it's worth reality check.
Yeah, isn't that how MicroPython was started at all? And ain't it cool, despite "may not work in the end anyway" to build something which does work? And for playforms which "work anyway", that also allows to do much, much more.
Nobody talks about twisting for esp8266 which becomes obsolete in few years. That's why I think for example that it's non-productive for maintainers to work on things like #2891 - we already gave esp8266 a great boost from hardware support point of view, and the rest can be take forward by the community. And that happens, albeit there're still problems with code quality, so we should wait (and help with review) for that to gather more momentum. OTOH, ESP8266 Kickstarter was conceived exactly as a way to optimize MicroPython for low-heap systems (which also means optimizing it for all systems), which aren't becoming rare anytime soon. So, interesting things like optimizations start only now, and that's exciting! Thinking otherwise may as well mean to think "Maybe in few years, CPython will run on all those systems, so why bother (and have bothered) at all". |
Yes, sadly, it seems that we continue to come back, time and time again, to disagreements in the overall picture.
Wasn't the whole point of this PR to get pico-notes working on esp8266?
IMO this is exactly the sort of thing we (or I) should be working on: it's a core component that touches lots of internal things and needs someone who has a good understanding of the whole system to get it working. |
Looking thru a diffraction gap, yes, but the reasoning is exactly opposite: using particular esp8266 port, and particular applications running on it, to optimize MicroPython in general. |
I don't disagree, and I have it in my queue too, but due to fact that "esp8266 will be dead anyway (as any other hw)" and "[draft] patches exist", it's lower priority than making uPy better in general. |
SAMD: make the brownout detection level configurable per board
2 participants
Don't create copy of immutable object's contents until .write() is called
on BytesIO.