Investigate crate-size optimizations

[Rahix]

Right now we're calling form and rustfmt to split out the auto-generated modules into many files. I don't think this provides much value as nobody is going to look at those file anyway.

I have a suspicion that we could reduce the crate (download) size a bit by not calling form. For rustfmt on the other hand I am completely unsure whether it helps or not. svd2rust does generate a lot of unnecessary whitespace but I do not know whether rustfmt generates more or less (due to indentation). A rust minifier would of course be the best solution here ;)

[Rahix]

Looks like rustfmt makes files larger. Though if anyone wants to look at the source, a non-formatted file makes it completely impossible while a formatted but large single file (from not calling form) at least leaves some possibility to look at the code.

[tones111]

Another consideration in support of running the generated code through rustfmt are the documentation src links. I think it's more likely users would click through the source links to investigate the code then opening the file directly. Keeping this well formatted seems worth the space tradeoff. For similar reasons I'd be less excited about a minifier as it makes it harder to inspect.

Another potentially interesting approach I tried implementing a while back was moving the code generation to build-time (ref: stm32-rs/stm32-rs#5). Assuming the majority of the crate size is related to carrying the generated files for all supported processors, this approach only requires the input to the code generation. The tradeoff is that the user would need to generate for their target processor (once) during build.rs. One nice aspect was that it used svd2rust as a library and in specifying it as a build dependency could explicitly pin it to a specific version. This is a win as users could more reliably reproduce a build (since the svd2rust version you have installed and build with isn't specified anywhere).

edit: I found some additional discussion on build-time generation here: stm32-rs/stm32-rs#8

[Rahix]

Another consideration in support of running the generated code through rustfmt are the documentation src links. I think it's more likely users would click through the source links to investigate the code then opening the file directly. Keeping this well formatted seems worth the space tradeoff. For similar reasons I'd be less excited about a minifier as it makes it harder to inspect.

I was thinking people probably won't be looking at the generated sources much but maybe I am wrong about that.

Another potentially interesting approach I tried implementing a while back was moving the code generation to build-time.

That is actually what we were doing in the past, before avr-device was published to crates.io. The problem is that we have some python components in the build process which led to dependency hell for getting things to work. Especially for the AVR stuff I think the setup should be as simple as possible because this target attracts a lot of people with little experience in the embedded systems space (coming from Arduino C/C++).

If all the generation tools were rust though, I think the story would be different.

Anyway, I think for now let's keep it like it is until a complaint about crate size/anything else comes in.

[Outurnate]

Why not try generating more code at build time? atdf2svd, svd2rust, form and svdtools are all rust packages. With the exception of atdf2svd all crates have a lib.rs published, so calling out to them from build.rs should be straightforward. You could remove the Makefile and python dependency

[Rahix]

Reviving an old thread because I've just noticed how bad we have gotten in terms of size. While the crate archive is sitting at 2.6MB, the uncompressed sources have accumulated to a whopping 46MB:

46M		src/devices/
3.5M	src/devices/atmega4809
3.5M	src/devices/atmega4808
3.1M	src/devices/avr64du32
3.0M	src/devices/avr64du28
2.4M	src/devices/attiny1614
2.2M	src/devices/attiny816
1.9M	src/devices/atmega128rfa1
1.8M	src/devices/attiny404
1.7M	src/devices/attiny402
1.4M	src/devices/attiny202
1.4M	src/devices/atmega2560
1.4M	src/devices/atmega1280
1.1M	src/devices/atmega32u4
980K	src/devices/at90usb1286
952K	src/devices/atmega328pb
888K	src/devices/atmega128a
876K	src/devices/atmega64
868K	src/devices/atmega1284p
800K	src/devices/atmega324pa
788K	src/devices/atmega164pa
756K	src/devices/attiny841
748K	src/devices/atmega644
712K	src/devices/atmega8u2
680K	src/devices/atmega88p
680K	src/devices/atmega328p
680K	src/devices/atmega168
668K	src/devices/atmega48p
656K	src/devices/attiny828
644K	src/devices/attiny167
628K	src/devices/atmega16
592K	src/devices/atmega32a
572K	src/devices/attiny88
540K	src/devices/atmega8
520K	src/devices/attiny861
480K	src/devices/attiny84
472K	src/devices/attiny2313a
460K	src/devices/attiny85
448K	src/devices/attiny2313
444K	src/devices/attiny84a
440K	src/devices/attiny44a
296K	src/devices/attiny13a

[Rahix]

Why not try generating more code at build time? atdf2svd, svd2rust, form and svdtools are all rust packages. With the exception of atdf2svd all crates have a lib.rs published, so calling out to them from build.rs should be straightforward. You could remove the Makefile and python dependency

@Outurnate this is an interesting idea. Right now we are still using the old python version of svdtools so I don't know how difficult the switch to the Rust version would be. Then there is also the hacky manual patches we apply on top. And finally, with the current design, we also have codegen in the macro crate. We'd have to get rid of that entirely to make build-time generation work (wouldn't be a bad thing at all, though).

Regarding build-time generation of a peripheral-access-crate, do you know if there is any prior art of another crate doing this? So far I have only seen other crates that also generate before distribution.

Finally, we have to consider the implication of shipping the ATDF sources. I think there shouldn't be a problem with the license, etc. but we have to check.

[LuigiPiucco]

I've been taking a look at this crate since I found a few changes that could help with Rahix/avr-hal#457, and applying the suggestion to generate at build-time seems like it will help and also provide an opportunity to enhance the build-system as a whole. I can send a PR here to do the following:

• Update svdtools to 0.3.4 and switch to the rust version; This will require some changes to the yaml patches, which I've already done locally;
• Update svd2rust to 0.33.2; We already have Update to svd2rust 0.33.1 #155 for this, but I'll need it here, plus some other changes to accommodate it, thus my PR will include an update commit as well;
• Replace the Makefile with a build.rs; This entails using atdf2svd, svdtools and svd2rust as libraries via [build-dependencies], instead of the executables; Also, generate only the needed modules, not all of them every time;
• I think we may also get away with using the standard avr-unknown-gnu-atmega328 target for everything, using a feature-dependent piece in the build script to pass -mmcu to the linker based on the features; This would be particularly helpful over in the HAL, making the numerous JSON specs unneeded, and allowing us to not need a libcore per MCU. I'll try to include a preview of this.

I also have some changes to the ADC patches specifically, but I'll hold off on those; they rename the fields in a way that makes the split mux5 very obvious.

Finally, we have to consider the implication of shipping the ATDF sources. I think there shouldn't be a problem with the license, etc. but we have to check.

If the LICENSE file in the vendor directory is correct, they are Apache 2.0, which should give us no problems. We'd be distributing unmodified, since we only patch the SVDs, and those are our own derivative work.

And finally, with the current design, we also have codegen in the macro crate.

I think the cortex-m-rt crate changed after interrupts were implemented here, because https://docs.rs/cortex-m-rt/latest/cortex_m_rt/ indicates the macro implementation is not tied to knowing the device's interrupts; that knowledge is provided by the PAC, not the macro crate. If needed, I can update this too, though I assume we can generate the vector list via build.rs as well.

Edit: partial implementation in https://github.com/LuigiPiucco/avr-device/tree/size-opt; it doesn't yet have the build script part. I'll continue, probably tomorrow.

[Rahix]

I think we may also get away with using the standard avr-unknown-gnu-atmega328 target for everything, using a feature-dependent piece in the build script to pass -mmcu to the linker based on the features; This would be particularly helpful over in the HAL, making the numerous JSON specs unneeded, and allowing us to not need a libcore per MCU. I'll try to include a preview of this.

We can't get rid of the custom targets. The AVR microcontrollers each have a slightly different instruction set and the compiler needs to know what instructions it may generate.

[LuigiPiucco]

We can't get rid of the custom targets. The AVR microcontrollers each have a slightly different instruction set and the compiler needs to know what instructions it may generate.

You are correct about the ISA differences, the cpu key in the spec controls this. I thought I had found a way to circumvent that via passing -C target-cpu=<mcu>, but that seems to have slightly different behavior than the spec key, because, despite the Rust code being generated with the proper AVR revision, at the end it tries to link to a symbols.o file that the linker considers incompatible, failing. It works for passing -mmcu, but if we have to use a custom target anyway, that becomes pointless. So I'll drop it, but the other points about the build script still seem valid, I've (force-)pushed their implementation to the same branch as above.

In terms of size, I've run a comparison as follows:

# Package the crate, in this case it needs exactly one MCU feature enabled,
# since it performs a compilation check, but it shouldn't include that into
# the package.
$ cargo publish --dry-run --allow-dirty --features atmega328p,rt

# Compressed package size, I believe this is what the user has to download.
$ du -sh target/package/avr-device-0.5.4.crate
552K    target/package/avr-device-0.5.4.crate

# Uncompressed sources size, without the generated module and binaries.
$ du -sh target/package/avr-device-0.5.4 --exclude target/package/avr-device-0.5.4/target
5.0M    target/package/avr-device-0.5.4

# The generated module for atmega328p.
$ du -ah target/package/avr-device-0.5.4/target/avr-unknown-gnu-atmega328p/debug/build/avr-device-<some-hash>/out/pac
20K     target/package/avr-device-0.5.4/target/avr-unknown-gnu-atmega328p/debug/build/avr-device-<some-hash>/out/pac/generic.rs
528K    target/package/avr-device-0.5.4/target/avr-unknown-gnu-atmega328p/debug/build/avr-device-<some-hash>/out/pac/atmega328p.rs
552K    target/package/avr-device-0.5.4/target/avr-unknown-gnu-atmega328p/debug/build/avr-device-<some-hash>/out/pac/

Although there are probably many things to review since this is a pretty big change, I think it's worth investing some time into it. I'll open a PR later. It's in #157.