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

refactor: Avoid sending settings from worker threads #827

Merged
merged 3 commits into from
Oct 26, 2024

Conversation

marcustyphoon
Copy link
Collaborator

Each time the worker threads communicate results back to the main thread, they currently include the settings object on every character in the list of (up to) 50 character results, even though those settings will be the same for every result from a certain combination.

Back when the calculation was not run in a worker, this just meant that every character object had a pointer to the same settings object. But data sent via postMessage is cloned, meaning that every character object includes its own fresh copy of the identical settings object once it's sent back from a worker thread.

This moves the "attach settings object" behavior out of optimizercore, which is definitely run in a worker thread, and into optimizer.ts (the coordinator that manages state between workers and sorts results), which doesn't have to be in a worker (I'm currently undecided on whether it's better to or not).

This should save some postMessage traffic and memory/garbage collection, but it seems to have no meaningful effect on performance so far.

[draft previews]

Copy link
Contributor

github-actions bot commented Oct 20, 2024

Deployed preview build to Cloudflare!

Last commit: c090637
Preview URL: https://a7f7afcb.discretize-gear-optimizer.pages.dev
Branch Preview URL: https://character-settings-main-thread.discretize-gear-optimizer.pages.dev

@marcustyphoon marcustyphoon marked this pull request as ready for review October 26, 2024 01:52
@marcustyphoon marcustyphoon force-pushed the character-settings-main-thread branch from 89e51e3 to c090637 Compare October 26, 2024 01:52
@marcustyphoon marcustyphoon merged commit 3feceb7 into main Oct 26, 2024
3 checks passed
@marcustyphoon marcustyphoon deleted the character-settings-main-thread branch October 26, 2024 01:56
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

Successfully merging this pull request may close these issues.

1 participant