perf: compute standings tiebreaker values once and fix Buchholz complexity#7652
Open
Rathoz wants to merge 1 commit into
Open
perf: compute standings tiebreaker values once and fix Buchholz complexity#7652Rathoz wants to merge 1 commit into
Rathoz wants to merge 1 commit into
Conversation
Rathoz
force-pushed
the
standings-tiebreaker-single-compute
branch
from
ec216eb to
a67a167
Compare
hjpalpha
reviewed
Jun 12, 2026
| return '-' | ||
| end | ||
| return MathUtil.formatPercentage(self:valueOf(state, opponent), 2) | ||
| return MathUtil.formatPercentage(value ~= nil and value or self:valueOf(state, opponent), 2) |
Collaborator
There was a problem hiding this comment.
value ~= nil and is redundant
| function TiebreakerMatchWinRate:display(state, opponent) | ||
| return MathUtil.formatPercentage(self:valueOf(state, opponent), 2) | ||
| function TiebreakerMatchWinRate:display(state, opponent, value) | ||
| return MathUtil.formatPercentage(value ~= nil and value or self:valueOf(state, opponent), 2) |
| if value == nil then | ||
| value = self:valueOf(state, opponent) | ||
| end | ||
| return tostring(value) |
Collaborator
There was a problem hiding this comment.
return tostring(value or self:valueOf(state, opponent))
plus kick the if above
…exity Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rathoz
force-pushed
the
standings-tiebreaker-single-compute
branch
from
a67a167 to
cc9b45c
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes standings tiebreaker computation by avoiding repeated valueOf() calls during display rendering and by reducing Buchholz’s inner-loop cost via memoization and name-set lookups.
Changes:
- Compute each tiebreaker’s numeric value once per opponent/round and pass it into
display(...)to avoid recomputation. - Update default and overridden
display(...)implementations to accept the precomputed value. - Rework Buchholz enemy detection to use memoized enemy-name sets (with a fallback for renamed teams).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lua/wikis/commons/Standings/Parser.lua | Computes tiebreaker value once and passes it to display(...). |
| lua/wikis/commons/Standings/Tiebreaker/Interface.lua | Extends display(...) to accept an optional precomputed value. |
| lua/wikis/commons/Standings/Tiebreaker/Match/WinRate.lua | Updates win-rate display to use the passed-in value. |
| lua/wikis/commons/Standings/Tiebreaker/Match/Diff.lua | Updates display signature to accept the passed-in value. |
| lua/wikis/commons/Standings/Tiebreaker/Game/WinRate.lua | Updates win-rate display to use the passed-in value. |
| lua/wikis/commons/Standings/Tiebreaker/Game/Diff.lua | Updates display signature to accept the passed-in value. |
| lua/wikis/commons/Standings/Tiebreaker/Game/Rounds/Diff.lua | Updates display signature to accept the passed-in value. |
| lua/wikis/commons/Standings/Tiebreaker/Buchholz.lua | Improves Buchholz complexity using memoized enemy-name sets with rename fallback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+33
to
+36
| ---@param value integer? | ||
| ---@return string | ||
| function TiebreakerMatchWinRate:display(state, opponent) | ||
| return MathUtil.formatPercentage(self:valueOf(state, opponent), 2) | ||
| function TiebreakerMatchWinRate:display(state, opponent, value) | ||
| return MathUtil.formatPercentage(value ~= nil and value or self:valueOf(state, opponent), 2) |
Comment on lines
37
to
+41
| local games = TiebreakerGameUtil.getGames(opponent) | ||
| if games == 0 then | ||
| return '-' | ||
| end | ||
| return MathUtil.formatPercentage(self:valueOf(state, opponent), 2) | ||
| return MathUtil.formatPercentage(value ~= nil and value or self:valueOf(state, opponent), 2) |
Comment on lines
+39
to
+43
| ---@param value integer? | ||
| ---@return string | ||
| function StandingsTiebreaker:display(state, opponent) | ||
| return tostring(self:valueOf(state, opponent)) | ||
| function StandingsTiebreaker:display(state, opponent, value) | ||
| if value == nil then | ||
| value = self:valueOf(state, opponent) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related cuts to tiebreaker computation cost during standings parsing:
StandingsParser.calculateTiebreakerValuescalled bothvalueOfanddisplay, and the defaultdisplaycallsvalueOfagain (and the diff/winrate overrides recomputed their stats).display(state, opponent, value)now receives the already-computed value; the default implementation and all overriding tiebreakers (Match/Diff,Match/WinRate,Game/Diff,Game/WinRate,Game/Rounds/Diff) use it instead of recomputing.resolveTieForGroupcontinues to use storedtiebreakerValues/valueOfas before.Opponent.samein the inner loops. It now builds the enemy list as a name set viaOpponent.toName(with anOpponent.samefallback for renamed-team cases coming from match2 records) and memoizes the result per opponent entry, mirroring theFnUtil.memoizepattern inTiebreaker/Game/Util.Stored
tiebreakerValuesand display strings are unchanged.Based on
standings-tests-ai(#7647), whose tiebreaker and parser specs lock the expected values and display output.How did you test this change?
busted --run=ci: 605 successes / 0 failures, includingstandings_tiebreaker_spec.lua(buchholz, matchdiff/gamediff/rounddiff display cases) andstandings_parser_spec.lua(tiebreakerValues assertions) unchangedluacheckwithlua/.luacheckrc: 0 warnings on all touched files🤖 Generated with Claude Code