Skip to content

Conversation

GeoffreyFrogeye
Copy link
Contributor

@GeoffreyFrogeye GeoffreyFrogeye commented Apr 6, 2025

This reduce the size of the closures for the outposts.

Example for a machine that only uses the proxy outpost:

Closure size: 1172 -> 966 (5 paths added, 211 paths removed, delta -206, disk usage -1.7GiB).

I am not so sure this way of doing it is very clean, you need to copy the name of the outposts multiple times since it seems you can make an attr a package. Split packages doesn't sounds appropriate either, because then outpost depdends on the Python stuff even though it wouldn't actually change the binary.

Stuff that I removed because it didn't seem to be needed:

  • bin/proxy in the server package. IIUC the server doesn't call the proxy binary, it's just built-in.
  • doCheck = false I wanted to see for which binaries it was needed, turns out none?

Based on #46, so keeping as a draft until it and maybe #48 are merged. Just sounds like less work overall (and I can also test this MR as I require the proxy outpost). Tests are timing out, even though they aren't in my other MR so maaaybe it's something I did it's also timing out on other MRs.

@GeoffreyFrogeye GeoffreyFrogeye marked this pull request as draft April 6, 2025 20:12
@GeoffreyFrogeye GeoffreyFrogeye changed the title Draft: Split gopkgss Split gopkgs Apr 6, 2025
@Ma27
Copy link
Member

Ma27 commented Apr 7, 2025

Can't we split the stuff into multiple outputs and install only the outputs that are actually needed?
That way, we have a single build with everything, push the entire fiddling into install phase and still get the closure size reduction.

If you need any input on how to implement that, let me know.

@GeoffreyFrogeye GeoffreyFrogeye force-pushed the push-pollrpxtlxts branch 2 times, most recently from 41b79bb to e101c0e Compare April 8, 2025 23:31
@GeoffreyFrogeye
Copy link
Contributor Author

GeoffreyFrogeye commented Apr 8, 2025

So I tried split packages, it makes for a much cleaner code (see last push)! However, while the outpost doesn't contain the server binary, they still refer to some packages that are not actually needed but get included anyways, negating a good chunk of the size benefits. Here's a comparison from old method to new method:

Version changes:
[C.]  #1  libuv  1.48.0 -> 1.48.0, 1.48.0-dev
Added packages:
[A.]  #1  authentik-gopkgs          2025.2.3-proxy
[A.]  #2  goauthentik-web           2025.2.3
[A.]  #3  goauthentik-website-docs  2025.2.3
[A.]  #4  nodejs                    22.14.0
Removed packages:
[R.]  #1  authentik-proxy  2025.2.3
Closure size: 955 -> 959 (9 paths added, 5 paths removed, delta +4, disk usage +750.7MiB).

If the solution comes directly to you, let me know, otherwise I'll take some time to dig in the docs.

@Ma27
Copy link
Member

Ma27 commented Apr 11, 2025

Hmm, is it closure size that's increased or the binaries?
Maybe Go just links in everything into each binary.

@Ma27
Copy link
Member

Ma27 commented May 3, 2025

@GeoffreyFrogeye is there a reason this is still a draft?

@GeoffreyFrogeye
Copy link
Contributor Author

It's in draft because:

  • I'm not happy with the size reduction of the new method, nor think the old method is clean enough. I want to spend more time researching this.
  • Not feeling like researching now since I can't test code I write for this as-is in a meaningful way as I'm using 24.11 on my systems for now. Will be easier after 25.05 is released though.
  • I think it's best to merge first module: add basic proxy outpost service #46 which this depends on to reduce the number of rebases needed (maybe Add RAC outpost #48 too)

I'm still intending to work on this, but given this I think it's best to wait a bit.

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