Skip to content
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.

Sign up for GitHub

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

[YUNIKORN-2638] Simplify finalizeNodes and finalizePods #949

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[YUNIKORN-2638] Simplify finalizeNodes and finalizePods #949

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions pkg/cache/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1621,9 +1621,9 @@ func (ctx *Context) finalizeNodes(existingNodes []*v1.Node) error {
}

// convert the node list into a map
nodeMap := make(map[string]*v1.Node)
nodeMap := make(map[string]struct{}, len(nodes))
Copy link
Contributor

@wilfred-s wilfred-s Feb 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a readability perspective I would prefer nodeMap := make(map[string]bool, len(nodes)) and set true in 1626
Then at line 1631 the check simply becomes if !nodeMap[node.Name] {

Size (~5% more) and speed wise (not measurable unless the map has millions of entries) it does not make a difference when you consider what we use it for.

for _, node := range nodes {
nodeMap[node.Name] = node
nodeMap[node.Name] = struct{}{}
}

// find any existing nodes that no longer exist
Expand Down Expand Up @@ -1677,13 +1677,13 @@ func (ctx *Context) finalizePods(existingPods []*v1.Pod) error {
}

// convert the pod list into a map
podMap := make(map[types.UID]*v1.Pod)
podMap := make(map[types.UID]struct{}, len(pods))
for _, pod := range pods {
// if the pod is terminated finalising should remove it if it was running in register
if utils.IsPodTerminated(pod) {
continue
}
podMap[pod.UID] = pod
podMap[pod.UID] = struct{}{}
}

// find any existing pods that no longer exist
Expand Down