save a bit of space with some structure packing #6483
Conversation
lib/ftp.h
Outdated
| /* newhost is the (allocated) IP addr or host name to connect the data | ||
| connection to */ | ||
| unsigned short newport; /* connection to */ | ||
| char *newhost; /* this is the pair to connect the DATA... */ |
There was a problem hiding this comment.
These comments should be reversed but IMO are not needed at all because of the comment block above it
There was a problem hiding this comment.
Thanks, removing the per-field comments.
|
Summed up, all these tiny changes add up of course and I don't mind saving this memory at basically no cost. What's perhaps a little annoying is that the general idea to manually pack structs is to keep them in size order, in a large to small order but this bot only breaks that but it seems that we will add or change entries going forward that will alter this "optimized" state. @eriols, you mentioned you worked with the pahole tool to figure these out. Can we come up with a way to run that tool automatically somehow to check/update the struct status? |
include/curl/curl.h
Outdated
| unsigned int perm; | ||
| time_t time; /* always zero! */ |
There was a problem hiding this comment.
Good catch, I didn't see that. Correct, we must not change this!
There was a problem hiding this comment.
Thanks, dropping that one!
| @@ -1905,6 +1906,7 @@ struct Curl_easy { | |||
| int actions[MAX_SOCKSPEREASYHANDLE]; /* action for each socket in | |||
| sockets[] */ | |||
| int numsocks; | |||
| unsigned int magic; /* set to a CURLEASY_MAGIC_NUMBER */ | |||
There was a problem hiding this comment.
Spotting this change made me think and create #6484 instead
This is an attempt at saving a bit of space by packing some structs (using pahole to find the holes) where it might make sense to do so without losing readability. I.e., I tried to avoid separating fields that seem grouped together (like the cwd... fields in struct ftp_conn for instance). Also abstained from touching fields behind conditional macros as that quickly can get complicated.
eriols
force-pushed
the
struct_packing_patch
branch
from
1ca43e9
to
ca84251
Compare
|
@bagder I'll try to dig around to see if there are any automation possibilities within reach, but I figure it's a bit tricky to do. For projects that come in many flavors and for loads of architectures I figure this would mean running several parallel "packings" on different builds and then only applying the ones that do not mess up any other (important) configuration. |
|
A first step could be to just run it automatically and get some sort of daily summary or report or something, to see if we can learn from that and then see how we can proceed and improve. Probably on a chosen "default config" and platform or something. |
|
Right, so just something ultra-crude like |
|
Thanks |
This is an attempt at saving a bit of space by packing some structs (using pahole to find the holes) where it might make sense to do so without losing readability.
I.e., I tried to avoid separating fields that seem grouped together (like the cwd... fields in
struct ftp_connfor instance).Also abstained from touching fields behind conditional macros as that quickly can get complicated.
If this is at all useful (and I doubt it) I'm happy to squash; kept them separate for now to easy be able any unwanted ones.