Skip to content

Conversation

@jacob-keller
Copy link

The raid emulator analyzes an encounter by iterating over each party
member and analyzing the log. For encounters with many players, this can
take a long time, and appears to result in poor caching and JIT
performance from browser javascript engines.

Refactor the analyze() and analyzeFor() functions. Pass a list of party
member IDs to analyze at the same time. Modify the analyzeFor() function
to build a context for each player with its own popupText.

This allows iterating over the line once and analyzing it for each party
member instead of iterating over the entire log file many times. This
appears to behave more favorably for caching and JIT purposes.

To limit memory usage, slice the party member array by chunks of 24.
This will prevent extremely large encounters (such as 72-man CEs or
forked tower) from overwhelming the browser. Additionally, ensure that
all popupText always use the same regexCache. Remove the ability for the
popupText to ever allocate its own cache, and instead require it as a
constructor argument. Allocate a single regexCache in analyze() and pass
this in. This prevents accidentally generating multiple regex caches and
simplifies the logic for handling it all around.

This appears to be significantly faster on my setup for large
encounters. Potentially the batch size could be tweaked if its too large
or eliminated altogether if it turns out to be unnecessary. For now the
player context is a simple type but we could potentially refactor it
into a full class too.

The raid emulator analyzes an encounter by iterating over each party
member and analyzing the log. For encounters with many players, this can
take a long time, and appears to result in poor caching and JIT
performance from browser javascript engines.

Refactor the analyze() and analyzeFor() functions. Pass a list of party
member IDs to analyze at the same time. Modify the analyzeFor() function
to build a context for each player with its own popupText.

This allows iterating over the line once and analyzing it for each party
member instead of iterating over the entire log file many times. This
appears to behave more favorably for caching and JIT purposes.

To limit memory usage, slice the party member array by chunks of 24.
This will prevent extremely large encounters (such as 72-man CEs or
forked tower) from overwhelming the browser. Additionally, ensure that
all popupText always use the same regexCache. Remove the ability for the
popupText to ever allocate its own cache, and instead require it as a
constructor argument. Allocate a single regexCache in analyze() and pass
this in. This prevents accidentally generating multiple regex caches and
simplifies the logic for handling it all around.

This appears to be significantly faster on my setup for large
encounters. Potentially the batch size could be tweaked if its too large
or eliminated altogether if it turns out to be unnecessary. For now the
player context is a simple type but we could potentially refactor it
into a full class too.
@github-actions github-actions bot added needs-review Awaiting review raidemulator /ui/raidemulator module labels Jan 3, 2026
@jacob-keller
Copy link
Author

This is a new attempt at solving the problem as #911 but it behaves pretty well so far in my sampling of large encounters, and I think its a better solution. It probably needs a little more love to the playerContext stuff, I think breaking out the logic to create a popupText would be good. I'm also wondering if there is a better way of handling getCurLine() since its a closure that appears force some long lived references. Suggestions on how to clean that up are appreciated.

@jacob-keller
Copy link
Author

I tested with the batch size set to 72 with a forked tower pull log and it took a minute or so to finish analyzing it. I tried reducing the batch size to 1 to approximate the original logic, and it took about twice as long, but I need to see if I can get the browser to time how long it takes with this PR vs without, which I'll try to do tomorrow.

@valarnin
Copy link
Collaborator

valarnin commented Jan 3, 2026

but I need to see if I can get the browser to time how long it takes with this PR vs without, which I'll try to do tomorrow.

In RaidEmulator.ts, line 47:

    void this.dispatch('preCurrentEncounterChanged', this.currentEncounter);
    const start = performance.now();
    void this.currentEncounter.analyze().then(() => {
      console.log(`Analyzing encounter took ${performance.now() - start}`);
      void this.dispatch('currentEncounterChanged', this.currentEncounter);
    });

@valarnin
Copy link
Collaborator

valarnin commented Jan 4, 2026

Tested this PR and the changes look good in general. I don't have any content handy with more than 8 players, but testing various DoomtrainEx pulls shows the average analysis time for me drops from ~2500ms to ~1550ms.

I did notice a separate bug with CombatantTracker that is unrelated but which should result in more memory usage reduction, for some reason it's tracking random 4-character IDs as combatants, will need to debug to figure out why.

image

Applying this diff will result in a significant reduction in memory usage by not tracking data state information for triggers that do not modify the data state, as well as by not tracking properties that should not change from triggers (all default properties from RaidbossData).

This results in dropping the usage per perspective from 18515kb to 8188kb, etc (basically all of this memory usage is due to tracking actor positions):

image

I'd be curious to see what this does for memory usage for your use cases of >24 players.

diff --git a/ui/raidboss/emulator/data/PopupTextAnalysis.ts b/ui/raidboss/emulator/data/PopupTextAnalysis.ts
index c5cd5b3d0..f0de72249 100644
--- a/ui/raidboss/emulator/data/PopupTextAnalysis.ts
+++ b/ui/raidboss/emulator/data/PopupTextAnalysis.ts
@@ -1,4 +1,5 @@
 import { UnreachableCode } from '../../../../resources/not_reached';
+import { RaidbossData } from '../../../../types/data';
 import { EventResponses, LogEvent } from '../../../../types/event';
 import { Matches } from '../../../../types/net_matches';
 import { LooseTrigger, RaidbossFileData } from '../../../../types/trigger';
@@ -15,7 +16,7 @@ type ResolverFunc = () => void;
 export interface ResolverStatus {
   responseType?: string;
   responseLabel?: string;
-  initialData: DataType;
+  initialData?: DataType;
   finalData?: DataType;
   condition?: boolean;
   response?: undefined;
@@ -30,6 +31,30 @@ type EmulatorTriggerHelper = TriggerHelper & {
   resolver?: Resolver;
 };
 
+const dataPropsToExcludeMap: { [key in keyof RaidbossData]: boolean } = {
+  job: true,
+  me: true,
+  role: true,
+  party: true,
+  lang: true,
+  parserLang: true,
+  displayLang: true,
+  currentHP: true,
+  options: true,
+  inCombat: true,
+  triggerSetConfig: true,
+  ShortName: true,
+  StopCombat: true,
+  ParseLocaleFloat: true,
+  CanStun: true,
+  CanSilence: true,
+  CanSleep: true,
+  CanCleanse: true,
+  CanFeint: true,
+  CanAddle: true,
+};
+const dataPropsToExclude = Object.keys(dataPropsToExcludeMap);
+
 export class Resolver {
   private promise?: Promise<void>;
   private run?: ResolverFunc;
@@ -153,7 +178,7 @@ export default class PopupTextAnalysis extends StubbedPopupText {
           continue;
 
         const resolver = this.currentResolver = new Resolver({
-          initialData: EmulatorCommon.cloneData(this.data),
+          initialData: EmulatorCommon.cloneData(this.data, dataPropsToExclude),
           suppressed: false,
           executed: false,
         });
@@ -162,7 +187,11 @@ export default class PopupTextAnalysis extends StubbedPopupText {
 
         resolver.setFinal(() => {
           const currentLine = getCurrentLogLine();
-          resolver.status.finalData = EmulatorCommon.cloneData(this.data);
+          const finalData = EmulatorCommon.cloneData(this.data, dataPropsToExclude);
+          if (JSON.stringify(finalData) !== JSON.stringify(resolver.status.initialData))
+            resolver.status.finalData = finalData;
+          else
+            delete resolver.status.initialData;
           delete resolver.triggerHelper?.resolver;
           if (this.callback)
             this.callback(currentLine, resolver.triggerHelper, resolver.status, this.data);
@@ -186,7 +215,7 @@ export default class PopupTextAnalysis extends StubbedPopupText {
         }
         if (r !== false) {
           const resolver = this.currentResolver = new Resolver({
-            initialData: EmulatorCommon.cloneData(this.data),
+            initialData: EmulatorCommon.cloneData(this.data, dataPropsToExclude),
             suppressed: false,
             executed: false,
           });
@@ -198,7 +227,11 @@ export default class PopupTextAnalysis extends StubbedPopupText {
 
           resolver.setFinal(() => {
             const currentLine = getCurrentLogLine();
-            resolver.status.finalData = EmulatorCommon.cloneData(this.data);
+            const finalData = EmulatorCommon.cloneData(this.data, dataPropsToExclude);
+            if (JSON.stringify(finalData) !== JSON.stringify(resolver.status.initialData))
+              resolver.status.finalData = finalData;
+            else
+              delete resolver.status.initialData;
             delete resolver.triggerHelper?.resolver;
             if (this.callback)
               this.callback(currentLine, resolver.triggerHelper, resolver.status, this.data);

@jacob-keller
Copy link
Author

If we apply that I'll test dropping the slicing logic entirely and just doing everything at once, since its simpler and I think we'll be low enough mem it should no longer cause issues.

@valarnin
Copy link
Collaborator

valarnin commented Jan 4, 2026

If we apply that I'll test dropping the slicing logic entirely and just doing everything at once, since its simpler and I think we'll be low enough mem it should no longer cause issues.

Memory saving will vary depending on the encounter, this only helps when there's a lot of potential data being tracked which isn't usually the case for most content. A lot of the remaining memory usage for raidemulator is in CombatantTracker.

valarnin added a commit that referenced this pull request Jan 4, 2026
As mentioned in #917, noticed this bug while testing.
github-actions bot pushed a commit that referenced this pull request Jan 4, 2026
…ra (#919)

As mentioned in #917, noticed this bug while testing. ed2a29a
github-actions bot pushed a commit to ShadyWhite/cactbot that referenced this pull request Jan 4, 2026
github-actions bot pushed a commit to ShadyWhite/cactbot that referenced this pull request Jan 4, 2026
@jacob-keller
Copy link
Author

If we apply that I'll test dropping the slicing logic entirely and just doing everything at once, since its simpler and I think we'll be low enough mem it should no longer cause issues.

Memory saving will vary depending on the encounter, this only helps when there's a lot of potential data being tracked which isn't usually the case for most content. A lot of the remaining memory usage for raidemulator is in CombatantTracker.

True, but with the changes I did already I am not certain if the slicing helps anyways, so I'll test a couple things out.

@jacob-keller
Copy link
Author

I was worried that checking finalData by JSON would be expensive but it actually seems to have reduced the time taken. I also experimented with changing the batch sizes around, and it seems that at least for Forked Tower going above 24 makes it take longer. With a batch size of 24 it takes consistently 32 seconds, but increasing it either doesn't help or makes it slightly worse.

Comparing against main with just the timing addition it seems like I usually crash or at the very least it takes significantly longer at 52 seconds to parse, so its definitely a significant improvement.

@jacob-keller
Copy link
Author

Hm. Its unclear but I think it appears like we're somehow grabbing the wrong ID for LineEvent0x108 somehow, but I tried adding console logging and now I don't see it trigger reliably. My gut says that persisting to the database and then reloading somehow mucks up the line data.

@valarnin
Copy link
Collaborator

valarnin commented Jan 4, 2026

Hm. Its unclear but I think it appears like we're somehow grabbing the wrong ID for LineEvent0x108 somehow, but I tried adding console logging and now I don't see it trigger reliably. My gut says that persisting to the database and then reloading somehow mucks up the line data.

#919 fixed a bug with the initial parse logic. Click the re-parse button for the encounter, or delete and re-import it, to fix.

@jacob-keller
Copy link
Author

Ok. I think that leaves this cleanup ready to go once we're satisfied with testing. It still feels a bit slow for very large encounters, but its down to ~30-40 seconds instead of >1-2 minutes and crashing on my largest encounters. Especially with the fix to avoid the waste on the AbilityExtra lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review Awaiting review raidemulator /ui/raidemulator module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants