-
Notifications
You must be signed in to change notification settings - Fork 738
optimize nerdctl ps #4732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
optimize nerdctl ps #4732
Conversation
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code does not need to be a separate function, as it is called only once
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just create a scope with {}
Also please add the code comments about GC
| return nil, err | ||
| } | ||
| return prepareContainers(ctx, client, containers, cMap, options) | ||
| return prepareContainers(ctx, client, &containers, cMap, options) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
| } | ||
| li.Size = containerSize | ||
| } | ||
| (*containers)[i] = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here I want to let gc recycle the memory quickly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That behavior has to be explained in the code comments in the function declaration
Any benchmark results? |
|
1000 container speedup 1-2s @AkihiroSuda |
|
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 ) |
|
we can wait containerd/containerd#12846 or containerd/containerd#12862 to be merged. @AkihiroSuda |

speed up nerdctl ps we don't need call get container spec again.