linebender / xilem

An experimental Rust native UI framework
Apache License 2.0
3.14k stars 92 forks source link

Rewrite xilem_web to support new xilem_core #403

Closed Philipp-M closed 3 days ago

Philipp-M commented 1 week ago

This ports xilem_web to the new xilem_core.

There's also a lot of cleanup internally:

Downsides (currently, needs some investigation):

Due to more internal type complexity via associated types this suffers from https://github.com/rust-lang/rust/issues/105900. The new trait solver should hopefully mitigate some of that, but it seems currently it completely stalls in the todomvc example (not in other examples). The deep, possibly completely static composition via associated type-bounds of the view and element tree unfortunately can take some time to compile, this gets (already) obvious in the todomvc example. The other examples don't seem to suffer that bad yet from that issue, probably because they're quite simple.

I really hope we can mitigate this mostly, because I think this is the idiomatic (and more correct) way to implement what the previous API has offered.

One idea is to add a Box<dyn AnyViewSequence>, as every element takes a "type-erased" ViewSequence as parameter, so this may solve most of the issues (at the slight cost of dynamic dispatch/allocations).

Edit: idea was mostly successful, see comment right below.

I think it also closes #274

It's a draft, as there's a lot of changes in xilem_core that should be upstreamed (and cleaned up) via separate PRs and I would like to (mostly) fix the slow-compile time issue.

Philipp-M commented 1 week ago

but it seems currently it completely stalls in the todomvc example

Just to clarify, as I think there was already some misunderstanding, this is only true for the new (unfinished) trait-solver, not for the current one. The todomvc example takes without the new changes explained below, about 5s to compile in release mode vs 0.75s with current xilem_web. It'll be interesting to see how the new trait-solver will handle this, when it's finished, maybe we can convert it back to being completely static/stack-based again then (wishful thinking...).

So indeed the idea of type-erasing the ViewSequence mostly fixes the compile time issue, which makes sense, since it chops down the otherwise completely static composition of the view tree at each element boundary, since every DOM element takes a ViewSequence as parameter. But as expected this results in regression of either the performance (very slightly, less concerning than binary size at least) as well as binary size unfortunately. Compared to old xilem_web this binary size change is about 5-10% bigger (the lower end when it's compressed). Though I noticed, the bigger the app gets, the less of an issue this is (but we still need evidence of really big apps...)

I do think it's worth it to have this kind of regression, with that we are roughly at the same compilation times as before and IMO the other advantages (disregarding the actual port to new xilem_core) outweigh the slight regression. I also think that it (the compile times) should scale this way, as the static type composition will not grow "infinitely". With incremental compilation, probably also big apps will compile somewhat fast. But that needs to be proven with bigger apps than the todomvc example...

I've also started documenting xilem_web. So every item directly visible at in docs.rs should now at least hint what's going on.

Philipp-M commented 3 days ago

All of the dependent PRs are merged, it got another minor cleanup round, I think it's ready to merge now.