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 issue with client.emojis getter #6248

Closed
gc opened this issue Jul 31, 2021 · 1 comment · Fixed by #7039
Closed

Performance issue with client.emojis getter #6248

gc opened this issue Jul 31, 2021 · 1 comment · Fixed by #7039

Comments

@gc
Copy link

gc commented Jul 31, 2021

Please describe the problem you are having in as much detail as possible:
I did some profiling of my bot, and found DJS doing quite a large amount of cpu work here:

get emojis() {

I did a couple checks, and this is called many times, for example every time I add a reaction to a message, because those classes (e.g. Message, MessageReaction, ReactionCollector) will access client.emojis.resolveEmoji() or something like that. From my reading of the code it looks like its generating a new cache every single time you call it, i.e iterating over every emoji of every guild every time i react to a message.

Also, a quick little test I did was running this in my eval command +eval for (let i = 0; i < 20; i++) this.client.emojis; - which takes 8 seconds to finish, which doesn't seem right. unless for some reason I'm not understanding if its meant to be re-generated on every call or not - in either case, its using a lot of CPU, which really could be improved.

Include a reproducible code sample here, if possible:

For example, just a basic reaction is one place I saw this happening.

await message.react(MyEmojis.Smile);

Further details:

  • discord.js version: a08ce7dddb5f056128488392742495398f9e33b5
  • Node.js version: 14.17.3
  • Operating system: Windows
  • Priority this issue should have – please be realistic and elaborate if possible: Low

Relevant client options:

  • partials: ['USER', 'CHANNEL']
  • gateway intents: 'GUILDS', 'GUILD_MEMBERS', 'GUILD_MESSAGES', 'GUILD_MESSAGE_REACTIONS', 'DIRECT_MESSAGES','DIRECT_MESSAGE_REACTIONS','GUILD_WEBHOOKS'
  • other: none
@gc
Copy link
Author

gc commented Aug 19, 2021

I wasn't able to share a flamegraph because my v8 log was too big, but just to emphasize how much CPU this uses, here's screenshots of the processed log (just shows the getter using more CPU than almost anything else in the main process)
image

I'll probably migrate to buttons, as they're the more proper way of having users click on things - I'd assume discord devs prefer we use buttons. I did check and, although you can put custom emojis on buttons, they don't have this issue, because they use a util that specifically was made to not access the client:

* Resolves a partial emoji object from an {@link EmojiIdentifierResolvable}, without checking a Client.

I'm not sure how to fix this (add an option for caching them? find a way to not need to access client.emojis?). People dont want them cached because they want less memory usage, but people dont think about the cpu usage as much.

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

Successfully merging a pull request may close this issue.

4 participants