Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(backend): allow using environment variables to set settings #186

Merged
merged 6 commits into from
Mar 20, 2022

Conversation

ravenclaw900
Copy link
Owner

@ravenclaw900 ravenclaw900 commented Mar 20, 2022

Adds an actual configuration library, uses serde without serde_json, and uses jsonwebtoken instead of biscuit. Increases binary size quite a bit, so now optimizes some packages for size instead of speed.

@ravenclaw900 ravenclaw900 added the rust Pull requests that update Rust code label Mar 20, 2022
@ravenclaw900 ravenclaw900 mentioned this pull request Mar 20, 2022
24 tasks
@ravenclaw900
Copy link
Owner Author

That's odd, the size reported in the artifact download is about .5 MB off from the actual downloaded size of the binary.

@MichaIng
Copy link
Collaborator

Cleans up our code but adds significant dependency code. Do you think it's worth it? For config parsing, the benefit is obvious, but what is the benefit of using jsonwebtoken instead of biscuit?

@ravenclaw900
Copy link
Owner Author

ravenclaw900 commented Mar 20, 2022

jsonwebtoken is actually more lightweight, but it requires serde to use it. biscuit (actually a fork of jsonwebtoken) is larger, with more dependencies, but it doesn't need serde. Output of cargo-bloat before (note that this is compiled on my own machine, for gnu instead of musl):

 File  .text     Size Crate
 5.6%  16.6% 411.9KiB h2
 4.0%  11.7% 290.8KiB hyper
 2.9%   8.5% 211.4KiB core
 2.4%   7.2% 179.3KiB rustls
 2.3%   6.7% 166.1KiB ring
 2.1%   6.2% 153.4KiB dietpi_dashboard
 1.9%   5.7% 142.0KiB std
 1.4%   4.2% 105.3KiB tokio
 1.2%   3.4%  84.6KiB [Unknown]
 1.1%   3.3%  82.6KiB warp
 1.0%   2.8%  70.5KiB data_encoding
 0.8%   2.3%  56.8KiB http
 0.7%   2.0%  50.5KiB toml
 0.6%   1.9%  46.4KiB alloc
 0.6%   1.6%  41.0KiB addr2line
 0.5%   1.4%  34.4KiB psutil
 0.4%   1.2%  31.1KiB biscuit
 0.4%   1.2%  30.7KiB miniz_oxide
 0.3%   0.9%  21.8KiB rustc_demangle
 0.3%   0.8%  20.1KiB hashbrown
 3.1%   9.2% 228.8KiB And 51 more crates. Use -n N to show more.
33.9% 100.0%   2.4MiB .text section size, the file size is 7.2MiB

After (with size optimizations):

 File  .text     Size Crate
 5.4%  15.6% 368.9KiB h2
 4.2%  12.1% 286.6KiB hyper
 2.8%   8.1% 191.0KiB dietpi_dashboard
 2.7%   7.8% 184.4KiB core
 2.3%   6.6% 156.5KiB rustls
 1.8%   5.3% 124.3KiB ring
 1.7%   5.0% 117.8KiB std
 1.5%   4.4% 104.8KiB tokio
 1.3%   3.8%  89.8KiB warp
 1.2%   3.4%  80.6KiB alloc
 1.1%   3.2%  76.5KiB figment
 1.1%   3.2%  75.3KiB [Unknown]
 1.0%   3.0%  70.3KiB jsonwebtoken
 0.6%   1.9%  44.0KiB http
 0.6%   1.8%  42.3KiB toml
 0.6%   1.7%  41.0KiB addr2line
 0.5%   1.5%  34.4KiB psutil
 0.3%   0.9%  21.8KiB rustc_demangle
 0.3%   0.8%  18.4KiB serde_json
 0.3%   0.8%  18.3KiB miniz_oxide
 3.0%   8.5% 201.9KiB And 60 more crates. Use -n N to show more.
34.6% 100.0%   2.3MiB .text section size, the file size is 6.7MiB

Do you think it's worth it?

Honestly, now that I've done it, I'm not completely sure. I'll see the size comparison if I can find another config library that doesn't require serde and use that with biscuit. However, I might keep those optimizations on, while still disabling them for specific higher-performance crates.

@MichaIng
Copy link
Collaborator

MichaIng commented Mar 20, 2022

Ah, and since figment requires serde anyway, switching to jsonwebtoken makes it actually lighter. I understand.

Honestly, now that I've done it, I'm not completely sure.

Well, I mean the size of the assets with size optimisations is lower than latest main assets. Not sure whether the performance decrease for turning on size optimisation is in any way noticeable. Always good to have a look at alternatives, but while I did no performance tests, IMO it is fine as it is, simplifying (our) code for both, config parsing and token validation quite significantly.

@ravenclaw900
Copy link
Owner Author

ravenclaw900 commented Mar 20, 2022

Originally I was using the jwts library, which was very lightweight, however I'm now trying to use officially endorsed ones. One consolation, at least, is that we're still below the original Go binary size, when compressed. (1.4 MB vs. 2.3)

Actually, the compressed size is a lot less than I thought it would be, still smaller than 0.4.1. Plus, the inclusion of serde, without the monstrous serde_json, will allow us to now use other more popular and maintained crates.

@ravenclaw900 ravenclaw900 changed the title refactor(backend): switch to using serde and jsonwebtoken instead of biscuit feat(backend): allow using environment variables to set settings Mar 20, 2022
@ravenclaw900 ravenclaw900 enabled auto-merge (squash) March 20, 2022 22:24
@ravenclaw900 ravenclaw900 merged commit 9b80ade into main Mar 20, 2022
@ravenclaw900 ravenclaw900 deleted the serde-test branch March 20, 2022 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants