anp / moxie

lightweight platform-agnostic tools for declarative UI
https://moxie.rs
Apache License 2.0
828 stars 27 forks source link

Code bloat from multiple functions being generic over every closure #234

Closed anp closed 3 years ago

anp commented 3 years ago

In @ZetaNumbers' project, topo::call's generic instantiations take up over 20% of final binary size:

topo-code-bloat

I made it into a function as part of using #[track_caller] for everything, but this is obviously causing problems.

My initial thought was that we could make the function take a trait object reference, but that won't work because we take an FnOnce by-value.

I think it should be possible to make topo::{call,call_in_slot} back into a single macro that optionally accepts a slot for the call. Because it still uses track_caller it won't change the interfaces of any code "above" topo in the callstack, and because most code now uses #[topo::nested] the change shouldn't even result in a very large diff. It will still be a breaking change, though.

anp commented 3 years ago

One downside to this is that moving to functions allowed me to remove some technically-private-but-unstable machinery from the crate. #[docs(hidden)] it'll have to be.

anp commented 3 years ago

As pointed out on discord in earlier discussion, https://docs.rs/illicit/1.1.1/illicit/struct.Layer.html#method.enter is also generic and also takes the closure by-value. I think illicit::Layer::enter is probably the real source of bloat here, but could be wrong.

anp commented 3 years ago

From discussion on discord, current plan is to change illicit::Layer::enter to return a guard instead of taking a closure. In topo's internals we'll also need to rework things so that Scope::with_current/Scope::enter_child are no longer generic functions that vary between closure types.

If after that there's still significant bloat from topo::call, we'll try adding #[inline(always)] to it and call_in_slot, which should remove the remaining function stubs.

I'm filing a separate issue about regression tests for this so that we can keep from getting back into this scenario.