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
Optimize NormalizeScore for PodTopologySpread #95809
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ import ( | |
) | ||
|
||
const preScoreStateKey = "PreScore" + Name | ||
const invalidScore = -1 | ||
|
||
// preScoreState computed at PreScore and used at Score. | ||
// Fields are exported for comparison during testing. | ||
|
@@ -219,9 +220,10 @@ func (pl *PodTopologySpread) NormalizeScore(ctx context.Context, cycleState *fra | |
// Calculate <minScore> and <maxScore> | ||
var minScore int64 = math.MaxInt64 | ||
var maxScore int64 | ||
for _, score := range scores { | ||
for i, score := range scores { | ||
// it's mandatory to check if <score.Name> is present in m.IgnoredNodes | ||
if s.IgnoredNodes.Has(score.Name) { | ||
scores[i].Score = invalidScore | ||
alculquicondor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
continue | ||
} | ||
if score.Score < minScore { | ||
|
@@ -233,22 +235,14 @@ func (pl *PodTopologySpread) NormalizeScore(ctx context.Context, cycleState *fra | |
} | ||
|
||
for i := range scores { | ||
nodeInfo, err := pl.sharedLister.NodeInfos().Get(scores[i].Name) | ||
if err != nil { | ||
return framework.AsStatus(err) | ||
} | ||
node := nodeInfo.Node() | ||
|
||
if s.IgnoredNodes.Has(node.Name) { | ||
if scores[i].Score == invalidScore { | ||
scores[i].Score = 0 | ||
continue | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and check it as this,
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The score coming from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got you, thanks for the explanation. |
||
|
||
if maxScore == 0 { | ||
scores[i].Score = framework.MaxNodeScore | ||
continue | ||
} | ||
|
||
s := scores[i].Score | ||
scores[i].Score = framework.MaxNodeScore * (maxScore + minScore - s) / maxScore | ||
} | ||
|
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.
doing
for i, score := range scores
would make a copy of score for each element -- doesfor i := range score
do better? kinda premature optimisation, but worth a quick check :)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.
The object is so small that it doesn't really matter.