Skip to content

libcontainer: rm init() from capabilities_linux.go#2646

Closed
kolyshkin wants to merge 1 commit into
opencontainers:masterfrom
kolyshkin:caps-once
Closed

libcontainer: rm init() from capabilities_linux.go#2646
kolyshkin wants to merge 1 commit into
opencontainers:masterfrom
kolyshkin:caps-once

Conversation

@kolyshkin
Copy link
Copy Markdown
Contributor

Instead, use sync.Once to init the map when needed.

This should give a small speedup to operations which do not require
filling in the capabilities.

Instead, use sync.Once to init the map when needed.

This should give a small speedup to operations which do not require
filling in the capabilities.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@dims
Copy link
Copy Markdown
Contributor

dims commented Oct 13, 2020

+1 to this new pattern for initialization.

@cyphar
Copy link
Copy Markdown
Member

cyphar commented Oct 19, 2020

To be honest, I don't like this because func init() is kind of the recommended way of doing these kinds of initialisations in Go. And every container setup will end up calling initCapMap anyway.

You could argue that in principle operations other than runc create could be sightly faster -- but how much of a difference are we talking about? Not to mention that sync.Once actually takes locks internally and uses atomic load/store, so in the realm of micro-optimisations this actually makes container setup ever so slightly slower.

@kolyshkin
Copy link
Copy Markdown
Contributor Author

kolyshkin commented Oct 21, 2020

My concern was mostly about runc init which should be a small binary but it's not, partially because it includes libcontainer which does things like this.

Scratch that, this is part of runc init.

@kolyshkin kolyshkin closed this Oct 21, 2020
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.

3 participants