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

⚡️ Performance: Investigate using a Set to store actors, not an array #135

Open
3 tasks done
JoshuaKGoldberg opened this issue May 20, 2023 · 0 comments
Open
3 tasks done
Labels
status: needs investigation Further research required...? 🔎 type: feature New enhancement or request 🚀

Comments

@JoshuaKGoldberg
Copy link
Owner

JoshuaKGoldberg commented May 20, 2023

Bug Report Checklist

  • I have tried restarting my IDE and the issue persists.
  • I have pulled the latest main branch of the repository.
  • I have searched for related issues and found none that matched my issue.

Overview

Right now, emojisplosion internally uses an the Animator class to run the regular gameplay loop of telling EmojiActors ("actors") to animate. It stores actors in an array internally:

https://github.com/JoshuaKGoldberg/emojisplosion/blob/fe1ef216a89d6dd32bc5e752e203ceb66206c6d7/src/animator.ts#L11-L14

Whenever an actor "dies" (leaves the screen area), it's removed from that array with a .splice:

https://github.com/JoshuaKGoldberg/emojisplosion/blob/fe1ef216a89d6dd32bc5e752e203ceb66206c6d7/src/animator.ts#L52-L57

I wonder if that's actually inefficient? Now that all browsers have Sets, should we look into going with a Set for storage, so that it has O(1) removals?

Additional Info

...alternately, should we loop from the end of the actors array to the beginning (let i = actors.length - 1; i >= 0; i -= 1)?

Investigation needed!

Since this is a performance investigation, we'd need real data in multiple popular browsers (Chrome, Firefox, Safari) to indicate whether any approach is noticeably better or worse than others.

@JoshuaKGoldberg JoshuaKGoldberg added status: needs investigation Further research required...? 🔎 type: feature New enhancement or request 🚀 labels May 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs investigation Further research required...? 🔎 type: feature New enhancement or request 🚀
Projects
None yet
Development

No branches or pull requests

1 participant