Closed workingjubilee closed 1 year ago
This has different effects on the zero-feature build case for this repo.
However, your dependents are going to be enabling features, or else this crate will be roughly equivalent to another Rust file that is also very fast to compile:
When they add a feature, they will discard the incremental build cache again on the next compiler run. In many cases, people will have a compiling daemon build their crate periodically, which, unless they type very fast, will result in discarding and rebuilding this crate once for each feature they add, if memory serves. After doing this even 100 times, I expect, this will make node_modules
seem lightweight.
Meanwhile, each individual crate itself does not seem to suffer a notable impact from being built without cfg. This also affects the download of the crate and its index (which https://github.com/rust-lang/crates.io/issues/7269 mentions as containing all the features), which are also factors, especially in first-time addition of this crate or other fresh-slate builds.
Thank you for reaching out!
The first intentions with protecting every single icon behind a feature is to reduce wasm
bundle sizes. I believe web-sys
is also feature gating their structs for the same reason but I may be wrong 😅. After benchmarking on my machine, it seems that not using feature flags per icon and importing only 1 icon crate (i.e., icondata_fa
) bloats the bundle by 1.5MB!!
I'm afraid we will have to find an alternative solution to resolve this issue. I am fully aware of the stress this creates on Cargo
and crates.io
, and I am willing to look into solutions if you can think of any.
The web-sys crate is a direly undermaintained crate hacked together several years ago, when wasm tooling was much worse, yet expected to get better relatively quickly to the point of outmoding it in a few years, and then given up for other maintainers, who have since changed it further. There is no one who can meaningfully answer for its design choices anymore, so I will neither excuse them nor accuse them. I will say, however, that I think that any design choice that seems reasonable can become unreasonable if you add a few orders of magnitude to it.
From what I understand of how the compiler, linker, and other tools do dead code elimination, I believe that by creating a single type which references every single icon, the per-crate enum which then becomes an IconData, your crate's style virtually guarantees that any use of the enum with outside data that could make it take on any value would make dead code elimination almost impossible. That is assuming, of course, that the compilation tools adequately perform dead code elimination. It is plausible they do not.
If you wanted to assure that only used code is injected into the bundle, why not design your crate as a build tool, fundamentally? Via build.rs
or otherwise. You're already asking the programmer to list the icons they want, after all. This won't improve the initial build times, of course, but it will shift the burden to more appropriate locations and put more control in the hands of the actual user.
I believe there are three good solutions:
static
s instead of const
s (so that the data is guaranteed to be in a single place), possibly combined with #[link_section]
and possibly emitting metadata that makes it easier to track what gets actually used and put it into its own sections, and then DCE sections of statics quasi-manually using wasm-strip
or the like.I believe there are three good solutions:
- Work to significantly improve dead code elimination in general in the Rust/wasm ecosystem. May be slightly blue-sky.
I believe I am highly underqualified for this job, and I will leave it to someone who is more familiar with rust and compiler architectures.
- Shift the build towards the final package, by asking programmers to provide a file that enumerates the list of icons they want, stowed in a variable Cargo can track so that it can rerun-if-changed in the build.rs, that then builds the data in, so that it is more a "just-in-time" phenomenon.
This idea is for me the most compelling, and something I will try to achieve for the next release.
- Using
static
s instead ofconst
s (so that the data is guaranteed to be in a single place), possibly combined with#[link_section]
and possibly emitting metadata that makes it easier to track what gets actually used and put it into its own sections, and then DCE sections of statics quasi-manually usingwasm-strip
or the like.
Similar to the first option, I do not know enough about Rust's ABI and wasm
to undertake such a possibility.
I had another idea which I wanted to have your opinion on since you most certainly have more experience than me: How about a proc-macro that has the whole icon data-table and that inlines the data at compile time?
Thank you, your help means a lot!
I had another idea which I wanted to have your opinion on since you most certainly have more experience than me: How about a proc-macro that has the whole icon data-table and that inlines the data at compile time?
That seems reasonable. It's "morally equivalent" to the build.rs version, I think, since it's basically a big fat build step either way, but it may have different implications regarding deduplication across multiple dependents. I think your dominant use-case is likely to be just 1 or 2 crates using this library, so you should probably focus on whatever you feel has better user ergonomics.
Also, I believe either this PR or a similar solution of your own design is going to block the next release, due to this:
They aren't designing for more than 32K features maximum, which means that even if they granted you an exemption, this package would be dead in the water with only one or two more icon libraries added to it assuming you remained with its current design. Judging by their comments on the Rust Zulip, it seems to be because they basically only think this strategy is okay when it's about a thousand features (and even those require exemptions: the default limit is 300). I believe you want something that is capable of having 3 more icon libraries added to it, so that isn't going to work out for you.
You may want to consider accepting this PR so that you have the option of shipping a new release immediately, despite the binary size regression, and then doing the redesigns on top of it.
Currently, this library enables a
cfg(feature = "{name}")
for every single icon.Unfortunately, feature resolution the way Cargo does it is not, and cannot, be "fast" beyond a certain point: it is at minimum a time cost that is linear with the number of features. At its worst, the package resolution problem is often equated with bool-SAT. In addition, this adds several thousands of lines of code to each crate which must be lexed, parsed, and so on. As I recall, adding a feature is also going to invalidate the build cache for a crate.
My personal machine is very fast, but this still makes the builds notably faster.
check --all-features
build --all-features
I believe almost everything is in order, here, but I found difficulty testing the result meaningfully. One of the libraries did not finish downloading so I backed out the generated diffs involving it.