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.

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

Potential speed-up in sim/battle.ts #10629

Open
larry-the-table-guy opened this issue Oct 22, 2024 · 2 comments
Open

Potential speed-up in sim/battle.ts #10629

larry-the-table-guy opened this issue Oct 22, 2024 · 2 comments

Comments

@larry-the-table-guy
Copy link
Contributor

A considerable amount of time is spent in sim/battle.ts during npm run full-test.

Profiling

Here are some simple profiling results taken during npm run full-test, on a branch with #10616 (because that removes most of the randbats time, which makes ExhaustiveRunner stand out).

v8 prof

I'm seeing 30-40 seconds worth attributed to findEventHandlers and findPokemonEventHandlers (the latter is the bulk of the former) in a v8 prof.

ad hoc profiling (performance.now() and counting):

findEventHandlers
6.5 million calls
42368 milliseconds
Returned handlers length: frequency { '0': 6152113, '1': 335755, '2': 11933, '3': 188, '4': 10, '9': 1 }
94% of the time, no handlers.

findPokemonEventHandlers
38.5 million calls
37365 milliseconds
Returned handlers length: frequency { '0': 38137046, '1': 348380, '2': 13907, '3': 617, '4': 48, '5': 2 }
99% of the time, no handlers.

Note: with call counts this high, my caveman profiling has noticeable overhead.
I saw +4 seconds in findEventHandlers
and +10 seconds in findPokemonEventHandlers.
Nevertheless, findPokemonEventHandlers is a hotspot in ExhaustiveRunner during tests.


My suggestion

It's already ~1 microsecond per call. I don't think that can get radically faster in JS. So, call it less.
I think a strategy like modData in sim/dex.ts can work here: Anything that wants to add a handler must call a designated method, and that will set a flag which sim/battle.ts checks before looking for handlers. Conversely, when a handler is removed, the remove method will check if there are any handlers left and update the flag accordingly.

Then, during findEventHandlers and findPokemonEventHandlers:
If the flag is false, there are no handlers, return early. This is the 95-99% case.
If the flag is true, there are handlers, run the method as normal.

@Slayer95
Copy link
Contributor

Anything that wants to add a handler must call a designated method, and that will set a flag which sim/battle.ts checks before looking for handlers.

That was one of the intended purposes of Battle#onEvent(). In my mind, even, Battle could be a subclass of EventEmitter, or very similar, anyway. However, after I got Battle#onEvent() working for unit tests, I deprioritized that plan.

@Slayer95
Copy link
Contributor

Anyone interested in tackling this would benefit from reading #5439

Documentation rot fix: s/Effect/Condition

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants