Need to ponder: Binary bloat in `Modals`

[bunnie]

While adding bip39 dialog boxes to the system, I noted that a totally unrelated module that is not using BIP39 had its binary size increase.

Before: https://ci.betrusted.io/view/Enabled/job/zz-analyze-bloat/76/artifact/reports/root-keys.txt

  3 .text         0006b468  0002fe08  0002fe08  0001ee08  2**1
00076c24 00006470 t gam::modal::Modal::redraw

After: https://ci.betrusted.io/view/Enabled/job/zz-analyze-bloat/77/artifact/reports/root-keys.txt

  3 .text         0006ee12  00030120  00030120  0001f120  2**1
0004e7c0 00007f44 t gam::modal::Modal::redraw

There's a total increase by 14,762 bytes, and 6,868 of it ends up in the gam::modal::Modal::redraw routine alone.

Apparently what seems to be happening is all the redraw code for the new BIP39 modal is being included in every module that depends on Modal directly, even if it never instantiates one. This is I think because of this little gem:

#[enum_dispatch(ActionApi)]
pub enum ActionType {
    TextEntry,
    Bip39Entry,
    RadioButtons,
    CheckBoxes,
    Slider,
    Notification,
    #[cfg(feature="ditherpunk")]
    Image,
    ConsoleInput
}

enum_dispatch is pretty neat in that it allows dispatch to one of N elements that implement the same trait, but it does seem to mean that every item's code is also now compiled into your main loop and there's no opportunity for optimization.

It may be the case that there is no good way to reduce this, but, it does help explain partially why servers that don't use Images are blowing up in size when the ditherpunk feature is ungated.

[Hoverbear]

Inspecting the code generated by enum_dispatch (via cargo expand -p gam --lib modal > out.rs) it looks like it generates quite a bit of From implementations, I assume what's happening is these functions are somehow getting inlined into the redraw for some reason?

Running XOUS_SVD_FILE="/home/ana/git/betrusted-io/xous-core/precursors/soc.svd" RUSTFLAGS="--emit asm" cargo build -p gam --lib --target riscv32imac-unknown-xous-elf --release and checking target/riscv32imac-unknown-xous-elf/release/deps/gam-6e126c7aeed105db.s I found a _ZN3gam5modal5Modal6redraw17h950009f64eee5d01E which was about 1030 lines. Most of it is doing this enum dispatch stuff which seems to be inlined. What does yours look like? Maybe you could compare them?

[bunnie]

What is this cargo expand magic you speak of! This is really useful.

Yes, I think unfortunately enum_dispatch is one of those macros that takes an elegant-looking bit of syntax and hides a lot of ugly expansions inside which disallows optimizations. Normally Rust is extremely good at not including code that's not used, but I have come to realize the way the module is coded, of course it can't know that I will only ever use one of N modal flows -- it could receive any of the enum types going into the dispatch and so it has to include all N cases even if just one flow is needed.

I have to rethink this structure -- because the modals front end is rapidly turning into the equivalent of the user windowing toolkit and you don't want to statically link in every possible windowing primitive into every single one of your binaries (especially in a Xous model where you have so many micro-services), which is effectively what is happening right now.

[nworbnhoj]

The enum ActionType is a really tidy idiom (on the surface), and the binary bloat seems to be an inherent part of the idiom. But, on Precursor, the binary bloat has a very negative impact. I think we can still work with the enum_dispatch idiom by tightly restricting the memory associated with each option.

The enum ActionType Image option was the first glaring example of enum binary bloat as it holds 6 x 4096 byte Tiles in readiness for each graphics-server redraw. But the Image option can be easily refactored to simply contain 6 x usize tokens which are used to request the 6 x Tiles from some place else as required.

[bunnie]

I just had a thought about what might be the right way to restructure this.

The problem is that the data structure that manages the contents of the modals forms is kept on the client-side of the call. This means that also the code that interprets and renders it has to be stuck on the client side.

However, in general, the data structures are small -- just a couple of strings, typically.

I think maybe the right way to go about this is to keep enum ActionType for dispatching modals, but to shift the storage of the modal contents into the gam. The gam would have a mini database of modal subscribers, keyed by a 128-bit TRNG ID, so you would upload your data with a one-time use token and access it based on that API token.

This would reduce all of the client-side code to basically a small dispatch routine that handles tokens and packs the messages for transport across the IPC layer to the gam, and all the redraw code would be stored exactly once inside the gam server.

This would also mean that, for example, the image stuff that @nworbnhoj is working on could be transferred to the gam in an abstract form. That is, you would send either a reference to a URL or a PDDB file path for processing; or otherwise, at most, a dynamically allocated set of 6 tiles that eats up 24k of stack. That would also allow the image handling code to be a bit "heftier" because it's only stored once in the system, inside the gam. It does mean that server grows in size, but it's O(N) growth instead of the current O(N^2)-ish growth (each server that has a unique UX requirement has a copy of every other server's unique UX requirement).

A further improvement could be to redo the graphics drawing code so they aren't doing a hop through the GAM main loop to be dispatched to the gfx HAL -- could reference that object directly, saving one message pass and speeding up redraws.

It'd be a huge project to do this refactor, but, I think this might just be what has to be done.....thoughts?

[dumblob]

I am not a Rust fan but this sound rather overengineered compared to the problem we have here (binary bloat due to Rust's shortcoming).

Btw. it resembles the X11 architecture which the *nix world tries to get away from with Wayland and friends :wink:.

[jokeyrhyme]

I think maybe the right way to go about this is to keep enum ActionType for dispatching modals, but to shift the storage of the modal contents into the gam. The gam would have a mini database of modal subscribers, keyed by a 128-bit TRNG ID, so you would upload your data with a one-time use token and access it based on that API token.

Reminds me of Entity-Component-System approaches that are popular in game development, and I think this approach comes up in Rust quite often as a way to structure a global-ish shared state whilst satisfying the single-owner model (I'm no Rust expert, though)

Another approach I can think of could be to break up ActionType (and thus Modal) so that the heavier variations are separate from the lighter ones :shrug: (but the separate Modal's would still implement the same trait, to minimise impact on consumers)

[bunnie]

Somehow, @nworbnhoj's latest commits to ditherpunk have largely ameliorated the practical concerns at stake. I still haven't figured out exactly how it made most of the troubles go away.

Doesn't mean as a long-term thing this won't be a problem, but, at least in the short term it does de-prioritize this refactor.

[bunnie]

I'm pretty happy with where the bloat is right now, and there's a lot of stuff to do so I'm going to leave this open but leave the refactor for a later release.

[bunnie]

Did a little more poking at this tonight, and realized that the modals wrapper which has largely superseded direct GAM access insulates most of the system from bloat. It's still not perfect because a couple of the most secure crates do direct GAM access to avoid modals as a UI attack surface, but maybe the right way to handle that is to just split out the few secure ops into a subset of GAM primitives, instead of refactoring the whole stack to optimize for the minority use case.

Anyways, closing this for now as I think it's in an OK place. thanks everyone for the comments and ideas!