polaris
polaris copied to clipboard
agersant
Builds take too long
Polaris takes quite some time to build. It's not terrible, but I feel we could do better.
Some low-hanging fruit is:
- [x] using profile overrides to build proc macros in debug mode
- [ ] avoiding some of the dependency duplication (e.g. right now we're building two versions of
syn, each taking around 37 seconds to build - [ ] removing some of the less important proc macro dependencies (
function_name) comes to mind - [ ] making the TLS support optional (for people that use a reverse proxy)
- [ ] making the Last.fm support optional
- [ ] maybe something related to the testing code?
Filed https://github.com/polyfloyd/rust-id3/pull/34 for derive-builder and the old syn.
Thank you for kickstarting this effort. I've been unhappy with the build times for a while and I haven't had a chance to dig into it. A few notes/questions:
profile-override
Merged! That's a huge win!
avoiding some of the dependency duplication
The syn improvement to rust-id3 is going to help a lot, good catch. For the rest of the tree, I think it's going to get a big shakedown when the next Rocket finally is released (or one of my experiments with other framework ends well enough to switch). The current Rocket release pulls a very outdated version of hyper, which is full of bugs and also brings in a bunch of duplicate dependencies.
function-name
I think the 5 seconds are a reasonable trade-off versus the headache of giving tests unique db file names. I don't know if that's possible but maybe it could be compiled only with tests (which is where it's used)?
Making TLS optional
I think all the heavy crypto stuff gets pulled in by cookie regardless, which is very much non-optional. I would be open to a PR on this if I'm wrong though.
Making Last.fm optional
This one doesn't look like a big offender for compile time. If we were to do this, it would have to come with a new API endpoint to query supported features so that clients don't show UI for features that don't exist. (+corresponding polaris-web and android client work). Feels like a bad ratio of effort to savings.
As a side note, how did you generate the graph above?
I'm also trying to remove regex from rust-id3, but we'll certainly pull it off from other crates, so it won't help.
You can make the graphs on nightly with cargo build -Z timings.
Regarding the database names, can we use in-memory databases instead? I'm not sure if the in works (the database is deleted when the connection is closed).
Otherwise a combination of file! and line! might work.
We also use regex to match album art file names with the user-configured regex.
Good ideas for the database names, I think either solution could work!
Ah, I just noticed that the tests are using .sqlite files from the repository. My ideas don't work very well, then.
All the .sqlite files are throwaway that get created from scratch at the start of each test. I like having them linger because it makes it easy to troubleshoot test failures, but I haven't had to do that in a very long time so I could live without it.
Unfortunately we're still on that syn 0.15. It gets pulled in by Rocket's proc macros.
Build times have gotten significantly better since the migration away from rocket (+ various dependency updates). I also just submitted a change to compile diesel with support for only 16-column tables, which made a 20 seconds difference.
Timings for a clean debug build on my desktop:
