-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
tests/micropython: Make tests behave in low memory condition. #17955
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tests/micropython: Make tests behave in low memory condition. #17955
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #17955 +/- ##
=======================================
Coverage 98.38% 98.38%
=======================================
Files 171 171
Lines 22296 22296
=======================================
Hits 21937 21937
Misses 359 359 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Thanks for the PR! I ran Octoprobe on this PR, and it still fails sometimes with
|
This is odd. The error reported by Octoprobe is for an empty line in the source file, but that's right above a That said, I thought compile-time memory errors were caught by the test runner, but no worries. I can move the declaration for And if the remaining |
4510716
to
65779c6
Compare
The low memory errors you see on that specific board should be resolved now: https://reports.octoprobe.org/github_selfhosted_testrun_267/octoprobe_summary_report.html On that board memory was so low that preallocating code blocks and the work buffer didn't leave anything for the |
try: | ||
|
||
@micropython.viper | ||
def set_index(dest: ptr16, i: int, val: uint): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really don't think this needs to be within the try-except. The memory for the native code is allocated during the compile phase (as you originally guessed). The only thing that's allocated at runtime (after the point where run-tests.py
can detect an OOM failure) is the function-object wrapper, which is typically 1 GC block. The bytecode looks something like this:
...
00 MAKE_FUNCTION <address of native function>
02 STORE_NAME set_index
...
That's it. The small allocation is in the MAKE_FUNCTION
opcode, and potentially a resize of the globals dict in STORE_NAME
.
If this function can live where it was (outside the try-except) that would be preferable.
I think it's just the allocation of buffer
that's the biggest issue, it's 8k by my calculations. So that definitely needs to be within the try-except.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation! My understanding was that the native compilation step happened at a later stage than it really does and thus any error could be caught that way, more like a missing import could be a failure the code can possibly recover from.
This makes me assume that, if the system is critically low on memory for other reasons and a parse + native compilation step is triggered from the main script there is no chance for the running environment to recover from this situation. Or, in other words, there's no MemoryError
exception being raised and the system could possibly be caught in an unrecoverable bootloop.
If my understanding is correct, that's kind of scary, because all it takes is a memory preallocation in native code for example, driven by a data file whose header length got corrupted by a flash failure or by any other unexpected value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes me assume that, if the system is critically low on memory for other reasons and a parse + native compilation step is triggered from the main script there is no chance for the running environment to recover from this situation.
I don't think that's true. There should always be a way to catch all Python errors (unless it happens compiling boot.py
or main.py
...).
Wherever you trigger a parse+compile step, you should always be able to wrap that in a try-except.
65779c6
to
028dacf
Compare
I've resized the allocated buffers down to 5KiB each from the original 8KiB, keeping the same test behaviour. This should also help on boards that are even more memory constrained - in fact these tests are not marked as too large neither on an ITSYBITSY M0+ nor on an ESP8266 now! (See Octoprobe runs 276 and 277, respectively) |
This commit changes the "viper_ptr*_store_boundary" tests to make them fail more gracefully in low memory conditions. The original version of the tests compiled viper code blocks on the fly when it needed them, making them fail at runtime on some boards that do not come with enough memory for this test. This clashes with "run-tests.py"'s ability to look for a particular signature to mark tests as skipped due to not enough memory. Now compiled code blocks are generated at the beginning of the test inside an appropriate exception handler. In case of a memory error when pre-compiling a code block, the running test exits reporting a low memory condition to the test runner. This allows to have clean test runs on all platforms when it comes to viper pointer tests. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good now, I tested locally on ItsyBitsy M0 and these tests run and pass.
028dacf
to
b0fd007
Compare
Actually, they are still too large on ESP8266, the Octoprobe output is:
I also verified that locally. |
Summary
This PR changes the "viper_ptr*_store_boundary" tests to make them fail more gracefully in low memory conditions.
The original version of the tests compiled viper code blocks on the fly when it needed them, making them fail at runtime on some boards that do not come with enough memory for this test. This clashes with "run-tests.py"'s ability to look for a particular signature to mark tests as skipped due to not enough memory.
Now compiled code blocks are generated at the beginning of the test inside an appropriate exception handler. In case of a memory error when pre-compiling a code block, the running test exits reporting a low memory condition to the test runner. This allows to have clean test runs on all platforms when it comes to viper pointer tests.
Testing
Modified tests were run on an ESP8266 without any parameters to make sure they were reported as too large, and then with an appropriate prologue file to make sure the output is correct. The same tests were also executed on the Unix port on x64, just to be sure.
Trade-offs and Alternatives
I've tried to simplify certain parts of the test to counter the added complexity of generating blocks upfront, although it came with a test output data rearrangement to make it work. On the other hand, now those tests share most of their code except for the typed getters and setters.