Process,Config: use pointer receivers#5088
Conversation
rata
left a comment
There was a problem hiding this comment.
LGTM, thanks!
Just curious, is there some metric you care about that is improved with this or it's just a "Looking at the code I realized we are doing this, this is wasteful, let's avoid it"?
About the linter, do as you think it is best, fixing it doesn't seem hard... Although not sure I like the suggestion 😂
Rename a function parameter (containerId -> containerID) to avoid a linter warning: > var-naming: method parameter containerId should be containerID (revive) In many other places, including config.json (.linux.uidMappings and .gidMappings) it is already called containerID, so let's rename. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The Config type is quite big (currently 554 bytes on a 64 bit Linux) and using non-pointer receivers in its methods results in copying which is totally unnecessary. Change the methods to use pointer receivers. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The Process type is quite big (currently 368 bytes on a 64 bit Linux) and using non-pointer receivers in its methods results in copying which is totally unnecessary. Change the methods to use pointer receivers. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
I think I merely found one case just by looking at the code (don't remember which one), and then I asked claude code to identify similar cases (of a non-pointer receiver and the struct being copied is large). It found a few of which these two are the largest (its estimates of the size were slightly wrong, I re-checked those manually using
Fixed (and overall I like the suggestion since in config.json it's |
Config and Process types are quite big (currently 554 and 368 bytes on a 64 bit Linux),
so using non-pointer receivers in its methods results in copying which
is totally unnecessary.
Change their methods to use pointer receivers.