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: Show errored ports and Docker ports #7

Closed
wants to merge 4 commits into from

Conversation

quodlibetor
Copy link
Contributor

See the individual commits for easier-to-review work.

This is based on an older commit because the current main doesn't actually start the client afaict.

image

@DeCarabas
Copy link
Owner

What does:

the current main doesn't actually start the client afaict

mean?

If processes are running in a container then the fwd process
can't read their internal FDs without the CAP_SYS_ADMIN property
which is equivalent to sudo. Even with sudo, I think you need to do
a lot of work to be able to read them -- spawning a process within
the cgroup, doing work there, and then communicating back.

This just uses the docker api to populate some default ports, which
later get overwritten if fwd can find a native process.

The Docker port scan takes about 1.5ms, and the full port scan takes
40+ms, so this adds basically no overhead.
@DeCarabas
Copy link
Owner

My only reservation here is the fact that "bollard" appears to be a pretty big dependency. It only adds 5% to the aarch64 release binary size which is pretty good given how big bollard is and how many dependencies it has. (2290952 bytes vs 2406648 bytes, on my machine. Good job, I guess, rust compiler!)

Still, I wonder if we can do it more cheaply?

@DeCarabas
Copy link
Owner

Oh lol I was measuring on aarch64 which doesn't even include bollard because it doesn't talk to a docker daemon. Let me re-check on linux....

@DeCarabas
Copy link
Owner

OK, on my linux box, running rustc 1.80.0 (051478957 2024-07-21) with cargo build --release:

  • main branch @ 6335944: 2904696 bytes
  • these changes @ 05c0a0a (rebased onto my parsing fixes): 4840968 bytes

This is a 66% increase in binary size. Can we do better?

@quodlibetor
Copy link
Contributor Author

Very fair, I'll check.

now you can use the log crate and get messages in the frontend.
@quodlibetor
Copy link
Contributor Author

tl;dr fat lto seems to make the biggest difference, and I think running with this profile gets the best bang-for-buck:

[profile.small-s-strip_debuginfo-lto_fat]
inherits = "release"
strip = "debuginfo"
opt-level = "s"
lto = true

It gets fwd back down to 2,966,648 bytes (on current main (6335944) I get 1,843,880 with that profile, so the ratio stays the same but we're back to the same size). Docs for these options here.


I checked (rust 1.80) if disabling bollard's default features changes anything, and it doesn't seem like it:

fwd/target/x86_64-unknown-linux-gnu$ eza --long --no-time --no-user --bytes --sort size x86_64-unknown-linux-gnu/release/fwd-*
.rwxr-xr-x@ 5,029,536 release/fwd-no-default-features
.rwxr-xr-x@ 5,029,952 release/fwd-with-default-features

so I tried a few profiles, I ran:

for profile in $(rg -N -o 'small-[^]]*' Cargo.toml ) ; do
  echo "building $profile" | tee buildlog
  cross build --target x86_64-unknown-linux-gnu --quiet --profile "$profile" --bin fwd
done

and got:

$ eza --long --no-time --no-user --bytes --sort size x86_64-unknown-linux-gnu/*/fwd
.rwxr-xr-x@ 2,337,520 small-z-strip_symbols-lto_fat/fwd
.rwxr-xr-x@ 2,345,712 small-s-strip_symbols-lto_fat/fwd
.rwxr-xr-x@ 2,770,776 small-s-strip_debuginfo-lto_fat-codegen1/fwd
.rwxr-xr-x@ 2,966,648 small-s-strip_debuginfo-lto_fat/fwd
.rwxr-xr-x@ 2,966,648 small-s-strip_no_____-lto_fat/fwd
.rwxr-xr-x@ 3,096,720 small-z-strip_no_____-lto_fat/fwd
.rwxr-xr-x@ 3,655,400 small-_-strip_debuginfo-lto_fat/fwd
.rwxr-xr-x@ 3,940,072 small-z-strip_symbols-lto_no/fwd
.rwxr-xr-x@ 3,949,680 small-s-strip_debuginfo-lto_thin-codegen1/fwd
.rwxr-xr-x@ 4,690,608 small-s-strip_debuginfo-lto_thin/fwd
.rwxr-xr-x@ 5,029,952 release/fwd
.rwxr-xr-x@ 5,108,864 small-s/fwd
.rwxr-xr-x@ 5,843,992 small-z/fwd

with these profiles in Cargo.toml (based mostly on min-sized-rust):

[profile.small-z]
inherits = "release"
opt-level = "z"

[profile.small-s]
inherits = "release"
opt-level = "s"

[profile.small-z-strip_no_____-lto_fat]
inherits = "release"
opt-level = "z"
lto = true

[profile.small-s-strip_no_____-lto_fat]
inherits = "release"
opt-level = "s"
lto = true

[profile.small-_-strip_debuginfo-lto_fat]
inherits = "release"
strip = "debuginfo"
lto = true

[profile.small-_-strip_debuginfo-lto_thin]
inherits = "release"
strip = "debuginfo"
lto = "thin"

[profile.small-s-strip_debuginfo-lto_fat]
inherits = "release"
strip = "debuginfo"
opt-level = "s"
lto = true

[profile.small-s-strip_debuginfo-lto_thin]
inherits = "release"
strip = "debuginfo"
opt-level = "s"
lto = "thin"

[profile.small-s-strip_debuginfo-lto_fat-codegen1]
inherits = "release"
strip = "debuginfo"
opt-level = "s"
lto = true
codegen-units = 1

[profile.small-s-strip_debuginfo-lto_thin-codegen1]
inherits = "release"
strip = "debuginfo"
opt-level = "s"
lto = "thin"
codegen-units = 1

[profile.small-z-strip_symbols-lto_no]
inherits = "release"
strip = true
opt-level = "z"

@quodlibetor
Copy link
Contributor Author

quodlibetor commented Aug 2, 2024

oh but it'll be a few days before I can check what the actual performance of this is on the server, I've pushed up a commit that includes this profile in case you want to test.

@quodlibetor
Copy link
Contributor Author

I see you've also moved things around to make it easier to make things optional, would you like me to put the bollard dependency behind a docker feature?

@DeCarabas
Copy link
Owner

DeCarabas commented Aug 5, 2024

If you could split out the different features into different PRs that would be great. I have a stab at another approach for querying docker I would like to try if you don't mind... I should know if it will work today, or if we want bollard...

@DeCarabas
Copy link
Owner

I have pushed my version of docker support to the repository; do you think you could give it a try?

@quodlibetor
Copy link
Contributor Author

do you think you could give it a try?

I'll give it a shot but I'm gone for a week so it'll be a bit. The code looks reasonable, though. I mean, as reasonable as hand-writing a json parser can ever be 😂

@quodlibetor quodlibetor closed this Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants