optimize nerdctl ps - #4732
optimize nerdctl ps#4732ningmingxiao wants to merge 1 commit into
Conversation
Sorry, something went wrong.
Signed-off-by: ningmingxiao <ning.mingxiao@zte.com.cn>
| } | ||
| } | ||
|
|
||
| func GetCommandFromSpec(spec typeurl.Any, trunc, quote bool) (string, error) { |
There was a problem hiding this comment.
This code does not need to be a separate function, as it is called only once
Sorry, something went wrong.
There was a problem hiding this comment.
since spec have much data, If I use it into a function, it may cause quickly gc (gc may happen after the function return)
if I use it into a for loop (gc may happen after every loop). @AkihiroSuda
Sorry, something went wrong.
There was a problem hiding this comment.
You can just create a scope with {}
Also please add the code comments about GC
Sorry, something went wrong.
| return nil, err | ||
| } | ||
| return prepareContainers(ctx, client, containers, cMap, options) | ||
| return prepareContainers(ctx, client, &containers, cMap, options) |
There was a problem hiding this comment.
why?
Sorry, something went wrong.
| } | ||
| li.Size = containerSize | ||
| } | ||
| (*containers)[i] = nil |
There was a problem hiding this comment.
?
Sorry, something went wrong.
There was a problem hiding this comment.
here I want to let gc recycle the memory quickly
Sorry, something went wrong.
There was a problem hiding this comment.
That behavior has to be explained in the code comments in the function declaration
Sorry, something went wrong.
Any benchmark results? |
Sorry, something went wrong.
|
1000 container speedup 1-2s @AkihiroSuda |
Sorry, something went wrong.
|
I find nerdctl ps use about 1g memory when container spec size is too big there are many env in spec (because user use enableServiceLinks:true and k8s has many services ) |
Sorry, something went wrong.
|
we can wait containerd/containerd#12846 or containerd/containerd#12862 to be merged. @AkihiroSuda |
Sorry, something went wrong.
Reviewers
At least 1 approving review is required to merge this pull request.
speed up nerdctl ps we don't need call get container spec again.