Optimize memory usage by reordering fields

[EwenQuim]

Just a dumb PR to reduce memory usage and improve performance by simply realigning struct fields.
I can't believe this has to be done manually—obviously, the compiler should handle it.

This also enables all govet linters in the CI (including the fieldalignment check).

We'll wait for #285 to see the improvement or lack thereof.

[dylanhitt]

TIL

[EwenQuim]

What do you think about the Makefile change @ccoVeille @dylanhitt ? It makes make fmt and then make optimize struct memory usage automatically!

[dylanhitt]

What do you think about the Makefile change @ccoVeille @dylanhitt ? It makes make fmt and then make optimize struct memory usage automatically!

Yes I like it. Will say maybe we shouldn't be installing things on people machines so implicitly. I tend to use like a make init or make deps. For instance I would which like how you're doing and then if fail i would print a statement to the user saying x needs to be installed and make init will do it for you.

[ccoVeille]

I don't think anything like this is needed.

You are building an API service, with OpenAPI features.

Here it's about optimizing bytes of memory on a framework, that stay still between 2 API calls.

If someone needs memory efficient solution, I'm afraid to say they will never use a framework like fuego, I mean any framework.

I would like to discourage you to go in this direction that is making the struct less readable. Fields are no longer grouped by feature. The maintenance will be a pain.

For me it's pure "oh it's good feature we should try"

So unless you have a benchmark that shows a change of at least 20% I'm term of speed or memory consumption, I would say NOGO

[dylanhitt]

wait a minute what? I'm sorry, but I don't follow.

Here it's about optimizing bytes of memory on a framework, that stay still between 2 API calls.

A fuego context and fuego response is gonna be created on every API call for some using this framwork? If i have an API with 1000 concurrent calls (which I do). You're allocating a context for all 1000 api calls.

[dylanhitt]

Like this struct alone could be created 10,000 times: https://github.com/go-fuego/fuego/pull/299/files#diff-c4dd82978a1a49bcd620aff72208bbd8114f09341d701e188c247ce3e3511117R126 in a matter of seconds.

[ccoVeille]

I'm sorry. I didn't expect my reply to lead to such reactions.

Is fuego highly optimized for ultra fast performance? I cannot tell.

The recent PRs I reviewed were about adding feature, not about looking for insane performance

I'm not saying the changes in this PR have no consequences at all. It's proven, known, documented, there are linters about it.

My point is "are they noticeable?" on fuego.

But will it has a consequence of server bills? A noticeable consequence on performance?

What I can see is the consequence on readability/maintainability.

You talked about an allocation on every http calls, but what is it compared the cost in memory of receiving and returning the HTTP messages ?

For me such optimization should be on compiler side. But it isn't unfortunately.

Also, such optimization provides an easy fix for small memory optimization. But this is far from being a way to improve a framework performance.

It's very good optimization when dealing with millions of data to sort in memory, yes.

But here, I'm doubtful. That's why I asked for benchmarks.

If you think the readability is still OK after such "optimization", why not.

[dylanhitt]

You talked about an allocatio on every http calls, but what is it compared the cost in memory of receiving and returning the HTTP messages ?

Again I'm not really understanding your point here either. This would be entirely on the user using fuego. We won't know their body or requestbody it's up to them to optimize their own structs. The best we can do is optimize what we can control when someone wants to squeak out some performance where they can.

[dylanhitt]

And to answer readability. No I don't really see an impact/maintenance over grouping struct fields. If we really cared so much about this I would much rather focus on better commits/concise PRs than I would the grouping of the struct. Git can at least tell a story.

[ccoVeille]

I apparently fail to explain why I think (hypothesis) it will have quite no consequences on the end user.

At the same time, I think that I'm getting you upset or disappointed, because of what I said, I'm sorry about that.

Also, you think it could (here another hypothesis) have noticeable consequences for the performance.

I conceeded there will be improvement, my point is about the fact I don't think the could be noticed.

I can be wrong, and it won't be a problem to me.

I asked for a benchmark, because I would like to get something that can be checked/compared.

Maybe @EwenQuim can tell what he thinks about it. Maybe he made a benchmark

[dylanhitt]

I'm not upset or disappointed. I'm just not understanding the point being made. I don't know what

Here it's about optimizing bytes of memory on a framework, that stay still between 2 API calls.

When I read that it sounds like you're saying is that "the code in fuego really only is responsible for 2 API calls so why optimize"?

I do agree that the majority of the optimization is really dependent on our users.

Fuego specifically fuego.Server really doesn't do much past creating a netHttpContext for every api call in regards to memory. However that is struct is allocated every time a fuego server route is used. So let's take our current benchmark on main. Specifically BenchmarkContext_Body/valid_JSON_body-10 and BenchmarkRequest/fuego_server_and_fuego_post-10 main gives us

BenchmarkContext_Body/valid_JSON_body-10                  389785              3055 ns/op            7258 B/op         32 allocs/op
BenchmarkRequest/fuego_server_and_fuego_post-10           207608              5718 ns/op            8273 B/op         69 allocs/op
BenchmarkRequest/fuego_server_and_std_post-10             307184              3850 ns/op            7489 B/op         46 allocs/op
BenchmarkRequest/std_server_and_std_post-10               391288              3054 ns/op            7137 B/op         29 allocs/op

vs this branch

BenchmarkContext_Body/valid_JSON_body-10                  400556              3012 ns/op            7242 B/op         32 allocs/op
BenchmarkRequest/fuego_server_and_fuego_post-10           207830              5660 ns/op            8257 B/op         69 allocs/op
BenchmarkRequest/fuego_server_and_std_post-10             300900              3814 ns/op            7489 B/op         46 allocs/op
BenchmarkRequest/std_server_and_std_post-10               371756              3028 ns/op            7137 B/op         29 allocs/op

So ~16 Bytes per request.

[ccoVeille]

Thanks for doing the benchmark.

[EwenQuim]

Don’t worry @ccoVeille, everything is all right!

I understand this approach might be cumbersome for readability in most cases, so I’ll remove changes for most structs and the linter. No more forced alignement!

However, it still makes sense to optimize memory usage for certain structs, especially netHttpContext, since it’s used on every request and remains hidden from the user’s perspective. But we'll take care of these manually. And yes, it's not critical.