Skip to content
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

MakeCode 2022 consumes more memory and some v4 programmes won't download anymore #4788

Open
microbit-carlos opened this issue Jul 1, 2022 · 69 comments
Assignees

Comments

@microbit-carlos
Copy link
Collaborator

Describe the bug
We've got several reports from users where their micro:bit MakeCode projects stopped working after MakeCode 2022 release date.

To Reproduce
Steps to reproduce the behavior:

  1. Download this hex file: microbit-fits-v4_does-not-v5.hex.zip
  2. Load it in https://makecode.microbit.org
  3. Click "Download"
  4. The error program too big by 6184 bytes! appears

The same hex file in https://makecode.microbit.org/v4 downloads correctly.

Rather than share one of the user programmes, I create this hex file by adding a bunch of extensions and just randomly dropping blocks on the workspace until it stopped working on MakeCode v5 and worked in v4. So the programme in itself will not make much sense.

Expected behavior

Existing programmes to still work.

Screenshots

image

micro:bit version (please complete the following information):

N/A

Desktop (please complete the following information):

  • OS: macOS 12
  • Browser: Chrome
  • Version: 103

Smartphone (please complete the following information):

  • Device: [e.g. iPhone6]
  • OS: [e.g. iOS8.1]
  • Browser [e.g. stock browser, safari]
  • Version [e.g. 22]

Additional context

makecode.microbit.org version: 5.0.6
Microsoft MakeCode version: 8.0.3
microbit runtime version: v2.2.0-rc6
codal-microbit-v2 runtime version: v0.2.40

@microbit-carlos
Copy link
Collaborator Author

microbit-carlos commented Jul 1, 2022

With the provided hex file in the MakeCode 2022 editor, if I then add the Data Logger extension, which is configured to be V2 only, then MakeCode successfully compiles/downloads the programme.

So the memory issue might mostly affect V1 (or at least is more easily encountered in V1), and as V1 DAL hasn't been updated vs MakeCode v4, it suggest all the additional memory comes from pxt updates.

@jaustin
Copy link
Collaborator

jaustin commented Jul 4, 2022

@abchatra @mmoskal we've had more reports of this than anything else and it's a regression for these users, so would like to see if we can investigate. Especially because it affects V2 users even though the constraint appears to be related to V1, really?

@abchatra
Copy link
Collaborator

abchatra commented Jul 5, 2022

@microbit-carlos @jaustin is it possible to create a repro without the extension? If you can do that, I would to figure out the exact checkin.

@microbit-carlos
Copy link
Collaborator Author

microbit-carlos commented Jul 5, 2022

It really is just copy and paste code until it just about reaches the limit in V4 and then load the hex in MakeCode 2022:
microbit-v4large.hex.zip

@abchatra
Copy link
Collaborator

abchatra commented Jul 5, 2022

Regression was between 4.1.5 & 4.16. I need to see what changed between these 2 versions.

makecode.microbit.org version: 4.1.5
Microsoft MakeCode version: 7.1.4
pxt-common-packages: v9.0.1
microbit runtime version: v2.2.0-rc6
codal-microbit-v2 runtime version: v0.2.32

makecode.microbit.org version: 4.1.6
Microsoft MakeCode version: 7.2.16
pxt-common-packages": v9.2.7
microbit runtime version: v2.2.0-rc6
codal-microbit-v2 runtime version: v0.2.32

@microbit-carlos
Copy link
Collaborator Author

While this issue has been focused on flash consumption, it looks like there is also a higher RAM consumption by MakeCode:

@riknoll
Copy link
Member

riknoll commented Jul 21, 2022

it appears that this commit is the culprit: microsoft/pxt-common-packages@9454016

@mmoskal
Copy link
Member

mmoskal commented Jul 21, 2022

Strange. The commit definitely looks like a noop.

@riknoll
Copy link
Member

riknoll commented Jul 21, 2022

well, it appears to be more than just that. delta debugging between the two versions @abchatra mentioned led me to that commit and checking out the commit before does fix the issue. however, reverting that commit on the stable branch that pxt-microbit points to does not fix the issue.

i've narrowed it down to libs/base and libs/radio, but there are no obvious suspects in the diffs between the old code and the new. checking out the old libs/base and libs/radio on the stable branch does reduce the code size quite a bit though.

@pelikhan
Copy link
Member

pelikhan commented Jul 22, 2022 via email

@mmoskal
Copy link
Member

mmoskal commented Jul 22, 2022

You could try to run this script https://github.com/microsoft/jacdac-stm32x0/blob/main/scripts/map-file-stats.js on the .map file before and after (requires local build)

@riknoll
Copy link
Member

riknoll commented Jul 22, 2022

Thanks @mmoskal, I'll give it a shot!

@riknoll
Copy link
Member

riknoll commented Jul 29, 2022

well it took me awhile to figure out how to get yotta to run locally. I ended up having to use Ubuntu 18.04!

Anyway, here are the things that changed between version 4.1.5 (left side of arrow) and 5.0.13 (right side of arrow)

RAM.libs/blocksprj/built/yt/source/radio/radio.cpp.o 1 -> 2
RAM.libs/blocksprj/built/yt/source/core/pins.cpp.o 10 -> 11
RAM.TOTAL 2692 -> 2694

DATA.libs/blocksprj/built/yt/source/pointers.cpp.o 1252 -> 1268
DATA.TOTAL 9098 -> 9114

TEXT.libs/blocksprj/built/yt/source/radio/radio.cpp.o 408 -> 508
TEXT.libs/blocksprj/built/yt/source/core/serial.cpp.o 544 -> 548
TEXT.libs/blocksprj/built/yt/source/core/codal.cpp.o 820 -> 818
TEXT.libs/blocksprj/built/yt/source/core/pins.cpp.o 1064 -> 1108
TEXT.ym/microbit-dal/source/microbit-dal.a 34882 -> 34996
TEXT.TOTAL 114738 -> 114968

FLASH.libs/blocksprj/built/yt/source/radio/radio.cpp.o 408 -> 508
FLASH.libs/blocksprj/built/yt/source/core/serial.cpp.o 544 -> 548
FLASH.libs/blocksprj/built/yt/source/core/codal.cpp.o 836 -> 834
FLASH.libs/blocksprj/built/yt/source/core/pins.cpp.o 1149 -> 1193
FLASH.libs/blocksprj/built/yt/source/pointers.cpp.o 1278 -> 1294
FLASH.ym/microbit-dal/source/microbit-dal.a 37391 -> 37475
FLASH.TOTAL 123836 -> 124082

@pelikhan
Copy link
Member

radio.on , radio.off ?

@pelikhan
Copy link
Member

This script is great! We should run it on every build :)

@riknoll
Copy link
Member

riknoll commented Aug 1, 2022

I tried reverting every file it listed as changed back to 4.1.5 and I'm still getting a "program too big by 5216 bytes". @mmoskal any pointers on how to interpret these results?

@mmoskal
Copy link
Member

mmoskal commented Aug 8, 2022

It could be that the size of compiled code increased - you could look at the assembly output in the editor - it has comments on top that specify sizes. To view the .asm file I think you have to disable the simulator otherwise it disappears very quickly. You could also diff the .asm but you may need to replace some/all numbers by a placeholder to remove "random" changes due to different AST node IDs etc.

I guess compiled code increase is more likely here - the 200 byte change above shouldn't be that big of a problem.

@AlasdairAtKitronik
Copy link

@microbit-carlos @abchatra Any update on this is it's definitely still an issue?

@abchatra
Copy link
Collaborator

Thanks for the reminder @AlasdairAtKitronik. I will update by EOW on the progress.

@abchatra
Copy link
Collaborator

abchatra commented Oct 19, 2022

@AlasdairAtKitronik https://makecode.microbit.org/stable should have a workaround with letting users to download on v2. Please take a look.

@abchatra
Copy link
Collaborator

abchatra commented Oct 19, 2022

See this for the exact dialog: #4824 (comment)

@jaustin
Copy link
Collaborator

jaustin commented Feb 9, 2023

So, I propose the following actions:

ASAP for users: Update /stable and /beta to:

  • Generate universal hex in this situation, but where the V1 program just generates an error code (926 please)
  • Allow web USB even after this error for V2 only (and show an error for V1 like we do for an incompatible extension. [1])

Hi @abchatra we're working through the 'in the background' bit, but not the ASAP bit.

Is there anyone that could look at this please? The experience is quite confusing for someone who hits this problem....

Given #4934 the priority of resolving this aspect of things has increased for me.

@abchatra would you like me to file a new issue just for the Ux side and keep this one for the investigation on reasons for the size increase?

@abchatra
Copy link
Collaborator

Can I just release the fix in the makecode.microbit.org/stable? Blocking the user from downloading is bit too much.

@abchatra
Copy link
Collaborator

/stable basically warns and lets user download.

@jaustin
Copy link
Collaborator

jaustin commented Feb 10, 2023

There are two issues with /stable, though it's definitely better than live

  • It doesn't let the user use web-usb
  • it generates an intel hex, not a universal hex, which means it will fail silently/weirdly on a micro:bit V1.

We really don't want V2 only non-universal hex files kicking around. Is it possible to generate a universal hex as if a V2-only extension had been added? (And the bonus is that I think this would also make web USB work)

@microbit-carlos
Copy link
Collaborator Author

microbit-carlos commented Feb 10, 2023

Summary

The current theory was that MakeCode v4 was using the pext/yotta:latest docker image, and MakeCode v5 is using pext/yotta:update-yotta3, with updated tooling versions, and this was causing the larger builds.

The tests show that the newer yotta docker image with a newer GCC version result in a smaller builds compared with the older image.

So, there must be something else causing the MakeCode v5 builds to consume more memory.

Testing the old Yotta Docker image

Okay, from my side I've been able to do some progress, mostly thanks to @aznhassan pxt PR and I am now able to build locally 🎉

There isn't a pxt release yet with the fix, so at the moment we need to clone the repo locally and link it to pxt-microbit.

@abchatra can a release be made soon? That'd save us a few CI steps we are adding to the CODAL repos to test with MakeCode.

I tried to create a MakeCode build with the pext/yotta:latest (old) vs pext/yotta:update-yotta3 (new),
however there is a bug in the old version of Yotta (v0.16.4) that means the build fails.
This issue wasn't being triggered before the Yotta servers were shut down, as we were fetching the targets from there. But now that we are fetching them GitHub we are hitting this bug and it stops the build.

The last Yotta release v0.20.5 fixes the issue, so to be able to test the old environment I had to create a docker image based on pext/yotta:latest and update Yotta inside it. This is the Dockerfile I've used to create a image to test pext/yotta:latest: https://gist.github.com/microbit-carlos/7069544b8d2d5ffc47dc5bd5a57c78f1

Test results

Okay, so finally with all that I was able to build MakeCode with different docker images, using the test programme at the top of the issue, and compare how big the same programme was in each MakeCode build.

  • Original issue: program too big by 6184 bytes
  • Current MakeCode deployment: program too big by 6196 bytes
  • Local pext/yotta:update-yotta3: program too big by 6256 bytes
  • Local pext/yotta:latest (modified with newer Yotta): program too big by 7280 bytes

So pext/yotta:update-yotta3 produces smaller builds than pext/yotta:latest.

The "current MakeCode" version under test was:
makecode.microbit.org version: 5.0.12
MakeCode version: 8.0.7
microbit runtime version: v2.2.0-rc6
codal-microbit-v2 runtime version: v0.2.40

How it was tested

These are the steps I used to build MakeCode locally.

First I made sure I don't have a global installation of the pxt cli tool:

sudo npm uninstall -g pxt pxt-core pxt-common-packages

Then I used the pxt and pxt-microbit main branches:

git clone https://github.com/microsoft/pxt-microbit.git
git clone https://github.com/microsoft/pxt.git
cd pxt
npm install
npm run build
cd ../pxt-microbit
npm install
sudo npm link ../pxt

At this point we have to modifying the pxt-microbit/pxtarget.json file.

To change the docker image used we replace the two mentions of pext/yotta:latest to whatever image we are testing.

For Yotta to be able to clone all the GitHub repositories we have to provide a GitHub Access Token. That was done by adding this environmental variable to the docker environment via the pxt-microbit/pxtarget.json file:

    "variants": {
        "mbdal": {
            "compile": {},
            "compileService": {
                "dockerArgs": [
                    "--env",
                    "YOTTA_GITHUB_AUTHTOKEN=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
                ]
            }
        },

At this point we can pxt build and serve:

npx pxt buildtarget --local
PXT_FORCE_LOCAL=1 PXT_RUNTIME_DEV=1 PXT_ASMDEBUG=1 npx pxt serve --local

And then loaded the hex file attached to the top comment: microbit-fits-v4_does-not-v5.hex.zip

@eanders-ms
Copy link
Collaborator

I dug into this a bit and found that somewhere between pxt-microbit releases v4 and v5, the micro:bit v1 hex start address gets pushed out by 6144 bytes:

pxt-microbit v4.0.24: .startaddr 0x35000
pxt-microbit v5.0.12: .startaddr 0x36800

I believe this reduces the space available to store user code by 6k.

Comparing v4 and v5:

  • Both depend on micro:bit DAL v2.2.0-rc6.
  • Only minimal changes to pxt-microbit's core cpp. Most notably, the addition of radio::on and radio::off. This adds maybe 100 bytes.

What is causing the start address to change? Signs point to something about the current toolchain used to build microbit v1 native code. This toolchain is producing different output than before, including a later start address.

To test this, in the v4 codebase, I made a tiny change to pxt-microbit's core cpp, just enough to trigger a rebuild by the compiler service. The resultant "hexlines" differed by only ~300 bytes, but the final .asm reports a start address bumped out by 5968 bytes. The asm start address is derived from a value embedded in this built hex blob. There my knowledge ends. I don't know how this address is determined by the toolchain. I will keep probing to get more visibility into the final hex layout.

So, the issue doesn't seem be with a specific MakeCode release, but with the toolchain used by the compiler backend.

If you'd like to test these variations yourself:

@riknoll
Copy link
Member

riknoll commented May 22, 2023

that matches my and @mmoskal's findings as well

@eanders-ms
Copy link
Collaborator

eanders-ms commented May 22, 2023

Interesting to note: The entrypoint for microbit v2 did not change between releases (all test cases report .startaddr 0x47000).

@jwunderl jwunderl removed their assignment May 22, 2023
@jaustin
Copy link
Collaborator

jaustin commented May 23, 2023

I think the question we really need to answer here, then, is what's the change in the toolchain?

As per #4788 (comment) when we build outside the MakeCode infrastructure (but with the MakeCode docker images) we see smaller builds from newer toolchains, not larger.

current theory was that MakeCode v4 was using the pext/yotta:latest docker image, and MakeCode v5 is using pext/yotta:update-yotta3, with updated tooling versions, and this was causing the larger builds.

[... more results from @microbit-carlos's post]

So pext/yotta:update-yotta3 produces smaller builds than pext/yotta:latest.

So is it that we've got the assumption wrong about which docker image is used when? Or has something else changed under the hood in the build system? Or perhaps looking at the number of bytes over is too crude to tell us what's going on? Perhaps updating the docker file/image with a newer yotta is also part of the story?

@eanders-ms
Copy link
Collaborator

I experimented building with different versions of the ARM GNU Toolchain and found that GCC versions 8 and 10 produce a build that gives back 2k to user code.

For each version of GCC, I had the compiler backend build an empty MakeCode project.

As a baseline, here are the v1 hex start addresses from MakeCode v4 and v5:

v4.0.24: 0x35000
v5.0.12: 0x36800

Here are the results of the experiments:

GCC Version start addr notes
5 N/A build environment was nonfunctional
6 0x36C00 1k degradation from MakeCode v5 baseline
7 0x36800 no change from v5 baseline. This was expected as this is the version of GCC we use currently
8 0x36000 2k improvement from MakeCode v5
9 0x36400 1k improvement from MakeCode v5
10 0x36000 2k improvement from MakeCode v5
11 0x36400 1k improvement from MakeCode v5
12 0x36400 1k improvement from MakeCode v5

This doesn't explain where the 6k went, but getting 2k back isn't bad.

Dockerfile: Dockerfile-yotta.zip

With GCC 10, compiling the original test program above (microbit-fits-v4_does-not-v5.hex) gives the error "program too big by 4276 bytes!"

@eanders-ms
Copy link
Collaborator

Thinking in a different direction... can we leverage the fact that most microbit programs don't use bluetooth to give space back to the user? By excluding the soft device hex in these instances and adjusting FLASH start accordingly, that could potentially free up a lot of space for user code, to the tune of 80k if I'm not mistaken. I could easily be missing something here.

@abchatra
Copy link
Collaborator

abchatra commented Jun 4, 2023

@finneyj

@eanders-ms
Copy link
Collaborator

eanders-ms commented Jun 5, 2023

This seems like a promising direction. In a test, I modified the yotta build to exclude softdevice and move the FLASH to start at 0x1000 (I chose 0x1000 in case something else wanted to exist at address 0 -- turns out nothing else did). In this configuration, the original test program (microbit-fits-v4_does-not-v5.hex) fits with 87k to spare.

This solution seems viable because in the MakeCode editor we can determine whether the project is using bluetooth before firing off the native code compilation request. This should allow us to invoke a yotta target that excludes softdevice in the non-bluetooth case.

On a technical note, because MakeCode caches built hex fragments by hash of the source they contain, we might need to inject a dummy function into the non-bluetooth source just to vary the hash.

@jaustin
Copy link
Collaborator

jaustin commented Jun 5, 2023

Hi @eanders-ms This is an appealing idea but for the foreseeable future with micro:bit, avoiding SoftDevice is not possible.

The issue is that even if a program doesn't explicitly use Bluetooth, we rely on SoftDevice to be able to enter pairing and BLE update mode. This is required for people using iPads, tablets and phones. We are seeing an increasing number of schools use, or want to use, micro:bit with iPads and it's important that the experience is as close as possible there to everywhere else.

So, basically SoftDevice needs to be in the flash always, even if the user is not using BLE as part of their program, so that we can update the program over BLE.

@finneyj and co have been looking at alternative BLE stacks to provide this bootloader mode, but right now SoftDevice is the most flash efficient option we are aware of that has the properties we need.

I think the primary areas I'm interested in for this are as in
#4788 (comment)

ASAP for users: Update /stable and /beta to:

Generate universal hex in this situation, but where the V1 program just generates an error code (926 please)
Allow web USB even after this error for V2 only (and show an error for V1 like we do for an incompatible extension. [1])
In the background:

Debug the size increase with @microbit-carlos and someone from @abchatra's team - for which the next step is getting a local reproduction of the MakeCode build where we can inspect the binaries created and be sure about the toolchain versions at issue

I still don't understand why the trend of size that MakeCode team are seeing is opposite to the trend Carlos sees (newer builds give smaller binaries for Carlos).

@finneyj
Copy link

finneyj commented Jun 6, 2023

Thanks @jaustin @eanders-ms.

Indeed, Softdevice is a hungry beast when it comes to FLASH and RAM memory... We do have a yotta target configured to run with Softdevice, but as Jonny said, that would leave a lot of our users stranded that rely of Bluetooth for programming...

Early indications from student studies indicate that we could get back upward of 40K of flash from switching BLE stack. This is not a trivial undertaking though - it'll take a lot of dev+test time to make sure we have a comparably good experience. BLE is a remarkably twitchy protocol, with a lot of edge cases where different stacks behave differently... There are also some security features that we currently enable that might not be available in the new stacks.

Although well worthy, until now it's not really been flagged as a high enough priority...

@eanders-ms
Copy link
Collaborator

Thanks for the context @jaustin and @finneyj, makes sense.

Thinking about how to square my and Carlos' test results, I went back to the pext/yotta images to see what versions of the GNU toolchain they're using and found:

  1. pext/yotta:gcc4
    • yotta: 0.14.1
    • gcc: 5.2.1
  2. pext/yotta:latest (original)
    • yotta: 0.16.4
    • gcc: 6.2.1
  3. pext/yotta:latest (with Carlos mods)
    • yotta: 0.20.5
    • gcc: 6.2.1
  4. pext/yotta:update-yotta3:
    • yotta 0.20.5
    • gcc: 7.3.1

When I built the test program using yotta configurations 3 and 4 above and MakeCode v4.1.5, I got identical results for each (though the content of the hex files were not identical):

Image result
pext/yotta:latest program too big by 6136 bytes
pext/yotta:update-yotta3 (Carlos' version) program too big by 6136 bytes

I thought it suspicious that I couldn't create a working hex using the MakeCode v4.1.5/gcc v6 combination, because that combo works fine in production, or seems to. I wondered if in production we were not building anything at all and just picking up an old cached version of the hex. Searching the production hex cache, I found it, and interestingly, it was built on 6/21/2021 with gcc 5.2.1.

Looking at git history, in Sept 2021 we upgraded yotta, switching from the pext/yotta:gcc4 docker image to pext/yotta:update-yotta3 and going from gcc 5.2.1 to 7.3.1 in the process. I wonder if the the gcc upgrade caused the size increase, but due to hex caching the effect wasn't noticed until a change to the cpp was made to trigger a rebuild.

I think the next step is to create a yotta 0.20.5 + gcc 5 image and see what that yields. I should be able to try this later in the week.

@eanders-ms
Copy link
Collaborator

eanders-ms commented Jun 6, 2023

OK, I confirmed that GCC 5.4.1 + MakeCode 4.1.5 produce a test program that fits in memory by a whopping 8 bytes!

The bad news is it still doesn't work for MakeCode v5. It is too big by 1204 bytes.

image

I noticed the start address (where user code is added to the hex) jumped by 1024 bytes in the v5 build. I take this to indicate the cpp additions were enough to push it out to the next page boundary.

I think we have our culprits:

  • 5k loss due to GCC update
  • 1k loss due to CPP additions

Next steps:

  • Configure the build to use GCC 5 again
  • Discuss options for optimizing the cpp

@eanders-ms
Copy link
Collaborator

gcc5 toolchain for the microbit v1 build is live now. It is most noticeable in the beta channel, where you will get a new native hex built, while the release channel continues to use cached hexes (built with gcc7). gcc5 brings significant improvement to hex memory footprint, but doesn't get quite enough back for the test case here to work. Therefore keeping this bug open. We have additional work to do if we want to reduce the cpp footprint.

@microbit-carlos
Copy link
Collaborator Author

Thanks @eanders-ms, this is a great investigation on the memory consumption over different GCC toolchains!

Does that mean the current MakeCode live deployment builds V1 DAL with a docker image using gcc5, and V2 CODAL uses a docker image with gcc7?

If that's the case, could the https://github.com/microsoft/pxt-microbit/blob/master/pxtarget.json file be updated with these changes as well? Even if they are overwritten on the build server, it'd be really useful to have the pxtarget.json file to be as close as possible to the production configuration.

If you still have the different builds around, specifically the ELF files (usually built as a MICROBIT file without an extension, they can be identified because they start with the magic number 0x7F454C46, which is 0x7 'E' 'L' 'F' ) as then it'd be great to use something Bloaty to see where the extra flash is going:
https://github.com/google/bloaty

You can use this docker image directly, which might be quicker/simpler than building from source: https://github.com/carlosperate/bloaty-action/pkgs/container/bloaty#using-the-docker-image-to-run-bloaty-directly

@eanders-ms
Copy link
Collaborator

Oh yes, pxtarget.json should have been updated. Thanks for pointing that out. PR here: #5265

Curiously, the V2 CODAL build uses gcc 6. I left that unchanged for the time being.

Bloaty looks very useful. Our backend compiler does not produce ELF files, though it we could change that if needed. In the course of troubleshooting this, I developed a small utility to help me understand how a hex file loads into memory: https://eanders-ms.github.io/mb-hex-explorer/. It was educational for me, if nothing else. Though it would be useful to integrate something similar into our ci build and be able to catch regressions like this earlier.

@microbit-carlos
Copy link
Collaborator Author

Our backend compiler does not produce ELF files, though it we could change that if needed.

I thought it used the default CMake files from https://github.com/lancaster-university/microbit-v2-samples/ ? Are they being replaced, patched, or different ones used?

It'd be good for us to better understand how the backend compiler differs from the standard builds, as we have updated these in the past to accommodate some changes in the build system and/or configurations, and that might clash with what the backend compiler might be doing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests