Skip to content
This repository has been archived by the owner on Mar 9, 2022. It is now read-only.

AddProcessEnv has performance issue. #1044

Closed
Random-Liu opened this issue Feb 12, 2019 · 2 comments · Fixed by #1045
Closed

AddProcessEnv has performance issue. #1044

Random-Liu opened this issue Feb 12, 2019 · 2 comments · Fixed by #1045

Comments

@Random-Liu
Copy link
Member

Random-Liu commented Feb 12, 2019

AddProcessEnv uses a loop to override envs.

For a container with 26000 environment variables, this operation takes >40s......

We should use a map for this operation, like what docker is doing https://github.com/moby/moby/blob/8e610b2b55bfd1bfa9436ab110d311f5e8a74dcb/container/env.go#L9.

We should audit other functions we use from runtime-tools to avoid similar performance issue.

@Random-Liu
Copy link
Member Author

I think the other "for" loops in https://github.com/opencontainers/runtime-tools/blob/master/generate/generate.go are fine.

@mikebrow
Copy link
Member

	env := fmt.Sprintf("%s=%s", name, value)
	for idx := range g.Config.Process.Env {
		if strings.HasPrefix(g.Config.Process.Env[idx], name+"=") {
			g.Config.Process.Env[idx] = env
			return
		}
	}

oh my

thaJeztah added a commit to thaJeztah/containerd that referenced this issue Feb 13, 2019
…8a816bf964

includes backports of:

- containerd/cri#1042 Set /etc/hostname (fixes containerd/cri#1041)
- containerd/cri#1045 Fix env performance issue (fixes containerd/cri#1044)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/containerd that referenced this issue Feb 13, 2019
…06d9200e95

includes backports of:

- containerd/cri#984 filter events for non k8s.io namespaces (resolves firecracker-microvm/firecracker-containerd#35)
- containerd/cri#991 Remove container lifecycle image dependency (fixes containerd/cri#990)
- containerd/cri#1016 [release/1.0] Specify platform for image pull (fixes containerd/cri#1015)
- containerd/cri#1027 Fix the log ending newline handling (fixes containerd/cri#1026)
- containerd/cri#1042 Set /etc/hostname (fixes containerd/cri#1041)
- containerd/cri#1045 Fix env performance issue (fixes containerd/cri#1044)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants