Simplify ticks, as the value is a constant#2377
Conversation
|
ping @kolyshkin @AkihiroSuda PTAL |
|
LGTM |
|
@kolyshkin LGTY? 🤗 |
| ) | ||
|
|
||
| var clockTicks = uint64(system.GetClockTicks()) | ||
| var clockTicks = system.GetClockTicks() |
There was a problem hiding this comment.
I'd rather drop system.GetClockTicks() entirely and have this as
const clockTicks = 100(with a comment that explains where it is coming from)
There was a problem hiding this comment.
Having it as const will help compiler to optimize the calculations, and I doubt there are any external users of system.GetClockTicks().
There was a problem hiding this comment.
It may still be used in some locations outside of runc, so we should probably be careful with removing system.GetClockTicks() altogether, but do you want me to put a const in this file (so that it doesn't depend on libnetwork/system ?
There was a problem hiding this comment.
Go API compatibility is not guaranteed across releases (at least until v1.0.0 GA), so we can safely remove system.GetClockTicks().
kolyshkin
left a comment
kolyshkin
left a comment
There was a problem hiding this comment.
suggested to make it const
|
Removed the function, and defined a local |
See for example in the Musl libc source code https://git.musl-libc.org/cgit/musl/tree/src/conf/sysconf.c#n29 This removes the cgo dependency for the system package. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
|
LGTM |
|
LGTM |
|
Thanks! |
3 participants
While checking dependencies, I stumbled on containerd/cgroups#12, which removes the CGO dependency from the containerd/cgroups package. Given that that code was based on the one in libcontainer, looks like we can make the same change here. See the discussion on that PR for further details
See for example in the Musl libc source code https://git.musl-libc.org/cgit/musl/tree/src/conf/sysconf.c#n29
This removes the cgo dependency for the system package.