Skip to content

Commit a4699d9

Browse files
ltogniolliLocalIdentity
ltogniolli
and
LocalIdentity
authored
Performance improvements for gem dropdown sort (#2184)
* Speed up gem dropdown DPS sorting and hover tooltips The dropdown hover tooltip was cleared and rebuilt every frame, and each rebuild runs a full build calculation - hovering one gem cost ~60 full calcs per second. It now rebuilds only when the hovered gem, the build's outputRevision, or the relevant default settings change (tooltip:CheckForUpdate), turning the per-frame cost into a one-time cost per hovered gem. This also implements the fastCalcOptions path that the DPS sort loop was already calling into: UpdateSortCache defines the options and CalcOutputWithThisGem forwards them to the misc calculator, which now accepts them as an optional third argument: - Accelerated environment reuse: per-gem calcs carry over the cached player/enemy/minion DBs and a persistent environment with the existing accelerate flags (nodeAlloc, requirementsItems, requirementsGems), instead of rebuilding unchanged state from scratch for every gem. - skipEHP: defence estimations (EHP/max hit) are skipped during sorting unless sorting by Effective Hit Pool. - fullDPSOnly: when sorting by Full DPS, only the calcFullDPS roll-up is computed; the main-skill pass whose output was discarded is skipped. All other GetMiscCalculator callers pass two arguments and are unchanged. Verified by comparing outputs of the fast and unaccelerated paths for every supportable gem on test builds: identical results across all sort fields. * Cache per-skill Full DPS results during gem dropdown sorting When sorting the gem dropdown by Full DPS, every candidate gem triggered a full calcFullDPS pass: one perform per Full-DPS-included skill, even though a support socketed into one group cannot affect most other skills. On builds with many included skills (e.g. several companions), this multiplied the cost of every dropdown open by the skill count. calcFullDPS now supports an optional per-skill result cache (specEnv.fullDPSCache), driven by diffing each skill's calculation inputs rather than by guessing what the candidate gem does: - During the base pass, each skill's harvested outputs are captured into a plain snapshot, together with its input references: the skill's own modifier list and the environment's "coupling surface" - the buffs, auras and curses every skill provides (buffList) and the exposure it can inflict, which are the channels through which one skill's gems can influence another skill's results. - On later calls, a skill whose own modifier list and the coupling surface are both unchanged merges its cached snapshot instead of recalculating. Anything that touches the surface (auras, exposure, buff-granting supports) conservatively recalculates every skill. - Modifier lists are compared by table identity, with a depth-limited structural fallback for the few modifiers that are reconstructed on every initEnv, which would otherwise defeat the diff. The harvest section of calcFullDPS is restructured into capture-then- merge to make snapshots replayable; merge order and semantics (sum vs maximum fields, Absolution count fix, entry naming) are preserved exactly. The cache is only used by the gem sort's fullDPSOnly path and is rebuilt with the calculator on every build change, so all other calcFullDPS callers and any stale-data concerns are unaffected. Adds TestFullDPSCache covering: cached results matching fresh calculations, local supports reusing other skills' results, exposure and aura supports forcing recalculation, the reconstructed-modifier corner case, candidate gem levels, and cache invalidation on build changes. * Simplify Full DPS harvesting into shared folding and uniform actor entries - Replace the repeated per-stat, per-actor merge blocks with a single merge spec and fold; pass snapshots hold uniform per-actor entries, with the per-actor differences (entry naming, pass counts, dot scaling and gating) resolved as plain data at capture time. - Unify the captured output fields into one harvestFields list. Stats previously not folded for minions never occur on test builds, and the mirage path is currently disabled, so results are unchanged. - Compare modifiers with tableDeepEquals, called in both directions since it only inspects the first table's keys; a one-sided match could let a modifier that gained a field produce a stale cache hit. * Fix cache for triggered skills * Indent * Fix merge issues --------- Co-authored-by: LocalIdentity <localidentity2@gmail.com>
1 parent 0d6108b commit a4699d9

3 files changed

Lines changed: 437 additions & 147 deletions

File tree

‎spec/System/TestFullDPSCache_spec.lua‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
describe("TestFullDPSCache", function()
2+
local calcsModule
3+
4+
before_each(function()
5+
newBuild()
6+
calcsModule = LoadModule("Modules/Calcs")
7+
end)
8+
9+
-- Two single-skill groups, both included in Full DPS
10+
local function buildTwoGroups()
11+
build.skillsTab:PasteSocketGroup("Spark 20/0 1")
12+
runCallback("OnFrame")
13+
build.skillsTab:PasteSocketGroup("Fireball 20/0 1")
14+
runCallback("OnFrame")
15+
local sparkGroup = build.skillsTab.socketGroupList[1]
16+
local fireballGroup = build.skillsTab.socketGroupList[2]
17+
sparkGroup.mainActiveSkill = 1
18+
fireballGroup.mainActiveSkill = 1
19+
sparkGroup.includeInFullDPS = true
20+
fireballGroup.includeInFullDPS = true
21+
build.mainSocketGroup = 1
22+
build.buildFlag = true
23+
runCallback("OnFrame")
24+
return sparkGroup, fireballGroup
25+
end
26+
27+
local function findGem(name)
28+
for _, gemData in pairs(build.data.gems) do
29+
if gemData.name == name then
30+
return gemData
31+
end
32+
end
33+
end
34+
35+
local function makeGemInstance(gemData, level)
36+
return {
37+
level = level or gemData.naturalMaxLevel, quality = 0,
38+
count = 1, enabled = true, enableGlobal1 = true, enableGlobal2 = true,
39+
gemId = gemData.id, nameSpec = gemData.name, skillId = gemData.grantedEffectId,
40+
gemData = gemData,
41+
}
42+
end
43+
44+
-- Run calcFullDPS, optionally with a cache, counting perform passes per skill name
45+
local function runFullDPS(cache, counts)
46+
local realPerform = calcsModule.perform
47+
if counts then
48+
calcsModule.perform = function(env, ...)
49+
realPerform(env, ...)
50+
if env.player and env.player.mainSkill then
51+
local name = env.player.mainSkill.activeEffect.grantedEffect.name
52+
counts[name] = (counts[name] or 0) + 1
53+
end
54+
end
55+
end
56+
local result = calcsModule.calcFullDPS(build, "CALCULATOR", {}, cache and { fullDPSCache = cache } or {})
57+
calcsModule.perform = realPerform
58+
return result
59+
end
60+
61+
-- Capture a base cache, then evaluate one candidate gem socketed into the group,
62+
-- returning the cached-path result, per-skill perform counts, and a fresh result
63+
local function evaluateGem(group, gemData, level)
64+
local store = { }
65+
runFullDPS({ store = store, capture = true })
66+
local slotIndex = #group.gemList + 1
67+
group.gemList[slotIndex] = makeGemInstance(gemData, level)
68+
local counts = { }
69+
local cachedResult = runFullDPS({ store = store }, counts)
70+
local freshResult = runFullDPS(nil)
71+
group.gemList[slotIndex] = nil
72+
return cachedResult, counts, freshResult
73+
end
74+
75+
local function assertClose(expected, actual, label)
76+
local diff = math.abs((expected or 0) - (actual or 0))
77+
local scale = math.max(math.abs(expected or 0), math.abs(actual or 0), 1)
78+
assert.is_true(diff <= scale * 1e-9, (label or "value") .. ": expected " .. tostring(expected) .. ", got " .. tostring(actual))
79+
end
80+
81+
it("returns identical results from the cached path and a fresh calculation", function()
82+
local sparkGroup = buildTwoGroups()
83+
for _, gemName in ipairs({ "Controlled Destruction", "Fire Exposure", "Purity of Fire", "Bleed II" }) do
84+
local gemData = findGem(gemName)
85+
assert.is_true(gemData ~= nil, "gem not found: " .. gemName)
86+
local cachedResult, _, freshResult = evaluateGem(sparkGroup, gemData)
87+
assertClose(freshResult.combinedDPS, cachedResult.combinedDPS, gemName .. " combinedDPS")
88+
assertClose(freshResult.TotalDotDPS, cachedResult.TotalDotDPS, gemName .. " TotalDotDPS")
89+
end
90+
end)
91+
92+
it("reuses the other skill's result for a local support", function()
93+
local sparkGroup = buildTwoGroups()
94+
local _, counts = evaluateGem(sparkGroup, findGem("Controlled Destruction"))
95+
assert.is_true((counts["Spark"] or 0) >= 1, "Spark should be recalculated")
96+
assert.is_true(counts["Fireball"] == nil, "Fireball should be served from the cache")
97+
end)
98+
99+
it("recalculates other skills when the support can inflict exposure", function()
100+
local sparkGroup = buildTwoGroups()
101+
local _, counts = evaluateGem(sparkGroup, findGem("Fire Exposure"))
102+
assert.is_true((counts["Spark"] or 0) >= 1, "Spark should be recalculated")
103+
assert.is_true((counts["Fireball"] or 0) >= 1, "Fireball must be recalculated when exposure enters the build")
104+
end)
105+
106+
it("recalculates other skills when the support changes the buff surface", function()
107+
local sparkGroup = buildTwoGroups()
108+
local _, counts = evaluateGem(sparkGroup, findGem("Purity of Fire"))
109+
assert.is_true((counts["Fireball"] or 0) >= 1, "Fireball must be recalculated when a buff/aura is granted")
110+
end)
111+
112+
it("caches skills whose supports rebuild level-scaled mods each pass (Minion Mastery)", function()
113+
local sparkGroup = buildTwoGroups()
114+
build.skillsTab:PasteSocketGroup("Skeletal Sniper 20/0 1")
115+
runCallback("OnFrame")
116+
local sniperGroup = build.skillsTab.socketGroupList[3]
117+
sniperGroup.mainActiveSkill = 1
118+
sniperGroup.includeInFullDPS = true
119+
local masteryData = findGem("Minion Mastery")
120+
assert.is_true(masteryData ~= nil, "Minion Mastery gem not found")
121+
table.insert(sniperGroup.gemList, makeGemInstance(masteryData))
122+
build.buildFlag = true
123+
runCallback("OnFrame")
124+
-- The GemSupportLevel mod granted by Minion Mastery is reconstructed every initEnv;
125+
-- the structural comparator must still recognise the sniper's inputs as unchanged
126+
local cachedResult, counts, freshResult = evaluateGem(sparkGroup, findGem("Controlled Destruction"))
127+
assert.is_true(counts["Skeletal Sniper"] == nil, "Skeletal Sniper should be served from the cache despite Minion Mastery")
128+
assertClose(freshResult.combinedDPS, cachedResult.combinedDPS, "combinedDPS with Minion Mastery in build")
129+
end)
130+
131+
it("candidate gem level changes the cached-path result like a fresh one", function()
132+
local sparkGroup = buildTwoGroups()
133+
local gemData = findGem("Controlled Destruction")
134+
local cachedLow, _, freshLow = evaluateGem(sparkGroup, gemData, 1)
135+
assertClose(freshLow.combinedDPS, cachedLow.combinedDPS, "combinedDPS at level 1")
136+
local cachedHigh, _, freshHigh = evaluateGem(sparkGroup, gemData)
137+
assertClose(freshHigh.combinedDPS, cachedHigh.combinedDPS, "combinedDPS at max level")
138+
end)
139+
140+
it("end to end: the misc calculator fast path matches the slow path", function()
141+
local sparkGroup = buildTwoGroups()
142+
local calcFunc = build.calcsTab:GetMiscCalculator()
143+
local fastOpts = { nodeAlloc = true, requirementsItems = true, requirementsGems = true, skipEHP = true, fullDPSOnly = true }
144+
for _, gemName in ipairs({ "Controlled Destruction", "Fire Exposure" }) do
145+
local gemData = findGem(gemName)
146+
local slotIndex = #sparkGroup.gemList + 1
147+
sparkGroup.gemList[slotIndex] = makeGemInstance(gemData)
148+
local slow = calcFunc(nil, true)
149+
local slowFullDPS, slowDot = slow.FullDPS, slow.FullDotDPS
150+
local fast = calcFunc(nil, true, fastOpts)
151+
sparkGroup.gemList[slotIndex] = nil
152+
assertClose(slowFullDPS, fast.FullDPS, gemName .. " FullDPS")
153+
assertClose(slowDot, fast.FullDotDPS, gemName .. " FullDotDPS")
154+
end
155+
end)
156+
157+
it("a stale cache is not reused after the build changes when recaptured", function()
158+
local sparkGroup, fireballGroup = buildTwoGroups()
159+
local gemData = findGem("Controlled Destruction")
160+
local before = evaluateGem(sparkGroup, gemData)
161+
-- change the other group's gem and re-evaluate; evaluateGem recaptures its own base,
162+
-- mirroring the calculator closure being rebuilt on every build change
163+
fireballGroup.gemList[1].level = 1
164+
build.buildFlag = true
165+
runCallback("OnFrame")
166+
local cachedResult, _, freshResult = evaluateGem(sparkGroup, gemData)
167+
assertClose(freshResult.combinedDPS, cachedResult.combinedDPS, "combinedDPS after build change")
168+
assert.is_true(math.abs(before.combinedDPS - cachedResult.combinedDPS) > 1e-6, "expected combinedDPS to change after lowering Fireball's level")
169+
end)
170+
end)

‎src/Classes/GemSelectControl.lua‎

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ local GemSelectClass = newClass("GemSelectControl", "EditControl", function(self
4747
end
4848
end)
4949

50-
function GemSelectClass:CalcOutputWithThisGem(calcFunc, gemData, useFullDPS)
50+
function GemSelectClass:CalcOutputWithThisGem(calcFunc, gemData, useFullDPS, fastCalcOptions)
5151
local gemList = self.skillsTab.displayGroup.gemList
5252
local displayGemList = self.skillsTab.displayGroup.displayGemList
5353
local oldGem
@@ -75,7 +75,7 @@ function GemSelectClass:CalcOutputWithThisGem(calcFunc, gemData, useFullDPS)
7575
gemInstance.gemData = gemData
7676
gemInstance.displayEffect = nil
7777
-- Calculate the impact of using this gem
78-
local output = calcFunc(nil, useFullDPS)
78+
local output = calcFunc(nil, useFullDPS, fastCalcOptions)
7979
-- Put the original gem back into the list
8080
if oldGem then
8181
gemInstance.gemData = oldGem.gemData
@@ -309,6 +309,10 @@ function GemSelectClass:UpdateSortCache()
309309

310310
local dpsField = self.skillsTab.sortGemsByDPSField
311311
local useFullDPS = dpsField == "FullDPS"
312+
-- Between iterations of the sort loop only the gem in this slot changes, so tree
313+
-- allocations and item/gem requirements can be carried over between calcs; EHP
314+
-- estimation is only needed when sorting by it
315+
local fastCalcOptions = { nodeAlloc = true, requirementsItems = true, requirementsGems = true, skipEHP = dpsField ~= "TotalEHP", fullDPSOnly = useFullDPS }
312316
local calcFunc, calcBase = self.skillsTab.build.calcsTab:GetMiscCalculator(self.build)
313317
-- Check for nil because some fields may not be populated, default to 0
314318
local baseDPS = (dpsField == "FullDPS" and calcBase[dpsField] ~= nil and calcBase[dpsField]) or (calcBase.Minion and calcBase.Minion.CombinedDPS) or (calcBase[dpsField] ~= nil and calcBase[dpsField]) or 0
@@ -317,7 +321,7 @@ function GemSelectClass:UpdateSortCache()
317321
sortCache.dps[gemId] = baseDPS
318322
-- Ignore gems that don't support the active skill
319323
if sortCache.canSupport[gemId] or (gemData.grantedEffect.hasGlobalEffect and not gemData.grantedEffect.support) then
320-
local output = self:CalcOutputWithThisGem(calcFunc, gemData, useFullDPS, fastCalcOptions, calcBase)
324+
local output = self:CalcOutputWithThisGem(calcFunc, gemData, useFullDPS, fastCalcOptions)
321325
-- Check for nil because some fields may not be populated, default to 0
322326
sortCache.dps[gemId] = (dpsField == "FullDPS" and output[dpsField] ~= nil and output[dpsField]) or (output.Minion and output.Minion.CombinedDPS) or (output[dpsField] ~= nil and output[dpsField]) or 0
323327
end
@@ -462,28 +466,32 @@ function GemSelectClass:Draw(viewPort, noTooltip)
462466
if self.hoverSel then
463467
local calcFunc, calcBase = self.skillsTab.build.calcsTab:GetMiscCalculator(self.build)
464468
if calcFunc then
465-
self.tooltip:Clear()
466469
self.tooltip.maxWidth = 500
467470
local gemData = self.gems[self.list[self.hoverSel]]
468-
local output = self:CalcOutputWithThisGem(calcFunc, gemData, self.skillsTab.sortGemsByDPSField == "FullDPS", nil, calcBase)
469-
local gemInstance = {
470-
level = self.skillsTab:ProcessGemLevel(gemData),
471-
quality = self.skillsTab.defaultGemQuality or 0,
472-
count = 1,
473-
enabled = true,
474-
enableGlobal1 = true,
475-
enableGlobal2 = true,
476-
gemId = gemData.id,
477-
nameSpec = gemData.name,
478-
skillId = gemData.grantedEffectId,
479-
displayEffect = nil,
480-
gemData = gemData,
481-
corruptLevel = self.skillsTab.defaultCorruptionLevel,
482-
corrupted = self.skillsTab.defaultCorruptionState == true,
483-
}
484-
self:AddGemTooltip(gemInstance)
485-
self.tooltip:AddSeparator(10)
486-
self.skillsTab.build:AddStatComparesToTooltip(self.tooltip, calcBase, output, "^7Selecting this gem will give you:")
471+
-- Rebuilding this tooltip runs a full build calculation, so only rebuild when the hovered gem or the underlying build changes
472+
if self.tooltip:CheckForUpdate(gemData, self.skillsTab.build.outputRevision, self.skillsTab.displayGroup, self.skillsTab.sortGemsByDPSField,
473+
self.skillsTab.defaultGemLevel, self.skillsTab.defaultGemQuality, self.skillsTab.defaultCorruptionLevel, self.skillsTab.defaultCorruptionState) then
474+
-- No fastCalcOptions here: the tooltip's stat compare shows defensive stats too, so it needs the full (unaccelerated) calc
475+
local output = self:CalcOutputWithThisGem(calcFunc, gemData, self.skillsTab.sortGemsByDPSField == "FullDPS")
476+
local gemInstance = {
477+
level = self.skillsTab:ProcessGemLevel(gemData),
478+
quality = self.skillsTab.defaultGemQuality or 0,
479+
count = 1,
480+
enabled = true,
481+
enableGlobal1 = true,
482+
enableGlobal2 = true,
483+
gemId = gemData.id,
484+
nameSpec = gemData.name,
485+
skillId = gemData.grantedEffectId,
486+
displayEffect = nil,
487+
gemData = gemData,
488+
corruptLevel = self.skillsTab.defaultCorruptionLevel,
489+
corrupted = self.skillsTab.defaultCorruptionState == true,
490+
}
491+
self:AddGemTooltip(gemInstance)
492+
self.tooltip:AddSeparator(10)
493+
self.skillsTab.build:AddStatComparesToTooltip(self.tooltip, calcBase, output, "^7Selecting this gem will give you:")
494+
end
487495
self.tooltip:Draw(x, y + height + 2 + (self.hoverSel - 1) * (height - 4) - scrollBar.offset, width, height - 4, viewPort)
488496
end
489497
end
@@ -508,7 +516,8 @@ function GemSelectClass:Draw(viewPort, noTooltip)
508516
if mOver and (not self.skillsTab.selControl or self.skillsTab.selControl._className ~= "GemSelectControl" or not self.skillsTab.selControl.dropped) and (not noTooltip or self.forceTooltip) then
509517
local gemInstance = self.skillsTab.displayGroup.gemList[self.index]
510518
local cursorX, cursorY = GetCursorPos()
511-
self.tooltip:Clear()
519+
-- Clear the update params too, so the dropdown hover tooltip above knows to rebuild
520+
self.tooltip:Clear(true)
512521
self.tooltip.maxWidth = 600
513522
if gemInstance and gemInstance.gemData then
514523
self:AddGemTooltip(gemInstance)

0 commit comments

Comments
 (0)