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

perf(NcEmojiPicker): decrease mounting time and memory by moving large constants initialization and storing out from instance's reactive data #4479

Merged
merged 1 commit into from Aug 31, 2023

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Aug 28, 2023

☑️ Resolves

  • Resolves: NcEmojiPicker mounts very slow.

📝 PR

i18n and emojiIndex are not a part of NcEmojiPicker's data. They never change and are not reactive state but just constants. However, they were defined in the data. So each time NcEmojiPicker instance is created:

  1. new copies of emojiIndex with constructor new EmojiIndex and i18n were created
  2. they became reactive: Vue recursively loops over each property, re-defined them and create associated Dep + Observer instances for reactivity.

While for i18n it's slightly noticeable (0.1ms finally), emojiIndex is very huge (with about 1MB data).

🖼️ Result

Moving constants from data outside the component decreases mounting time and allocating memory.

Metric 🐌 Before 🏎️ After
Mount time 40 - 42 ms per instance 0.90 - 0.98 ms per instance
Memory 1.325 MB per instance 0.073 MB per instance + 1 MB shared
Allocation sampling image image

40ms may sound nothing. But it multiplies on instances count. So mounting 100 pickers was 4 seconds of blocking time.

It also saves about 1.3 MB RAM per instance. Also sounds small, but it is 130 MB per tab with 100 pickers.

Example: Conversations in Talk with many messages with reactions.

Alternative solution

Initialization of emojiIndex was moved to setup with a check if it was initialized before. So the index is not initialized before the first instance mounting.

We can move it to the module, so index will be ready before the first instance is mounted.

P.S.

NcPicker is still super slow on opening and it is still > 1MB in the bundle.

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

@ShGKme ShGKme added enhancement New feature or request 2. developing Work in progress feature: emoji picker Related to the emoji picker component labels Aug 28, 2023
@ShGKme ShGKme self-assigned this Aug 28, 2023
@ShGKme ShGKme changed the title perf(NcEmojiPicker): move large constants from reactive data to variable [WIP] perf(NcEmojiPicker): move large constants from reactive data to variable Aug 28, 2023
@ShGKme ShGKme force-pushed the perf/emoji-picker-mount branch 2 times, most recently from 821f3ea to 5ad5179 Compare August 29, 2023 11:51
@ShGKme ShGKme changed the title [WIP] perf(NcEmojiPicker): move large constants from reactive data to variable perf(NcEmojiPicker): decrease mounting time and memory by moving large constants initialization and storing out from instance's reactive data Aug 29, 2023
@ShGKme ShGKme marked this pull request as ready for review August 29, 2023 11:52
@ShGKme ShGKme added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Aug 29, 2023
@ShGKme ShGKme added this to the 8.0.0 milestone Aug 29, 2023
@ShGKme
Copy link
Contributor Author

ShGKme commented Aug 29, 2023

/backport to stable7

Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slow laptop here:

Metric 🐌 Before 🏎️ After
Mount time 70 ms per instance 1.5 - 2.3 ms per instance
Memory +- same results +- same results

@ShGKme
Copy link
Contributor Author

ShGKme commented Aug 29, 2023

Memory +- same results +- same results

Don't forget to collect garbage before looking into heap size in the memory tab

@Antreesy
Copy link
Contributor

Don't forget to collect garbage before looking into heap size in the memory tab

I meant the same results as on your screenshots, only mounting time differs

…om instance's reactive data

Performance improvements:
- Mounting time from ~40 ms to ~1 ms per instance
- Memory per instance from 1.3 MB to (0.07 MB per instance + ~1 MB shared)

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme
Copy link
Contributor Author

ShGKme commented Aug 31, 2023

Rebased onto master. I'm not sure we need a performance test in the docs, so it's removed.

For history:

<template>
  <div>
      <button @click="key += 1">Re-mount {{ count }} NcEmojiPicker-s</button> See console output for the result
      <ul :key="key">
          <li v-for="i in count" :key="i">
              <span>#{{ i }}</span>
              <NcEmojiPicker @select="select" style="display: inline-block">
                  <NcButton>Select Emoji</NcButton>
              </NcEmojiPicker>
              <span>selected emoji: {{ emoji }}</span>
          </li>
      </ul>
  </div>
</template>
<script>
  let time;

  export default {
      data() {
          return {
              count: 100,
              emoji: '',
              key: 1,
          }
      },
      beforeUpdate() {
          time = performance.now()
      },
      updated() {
          const allTime = performance.now() - time
          console.log(`NcEmojiPicker ${this.count} instances re-mounting time: ${allTime.toFixed(2)} ms or ${(allTime / this.count).toFixed(2)} ms per instance`)
      },
      methods: {
          select(emoji) {
              this.emoji = emoji
          },
      },
  }
</script>

@ShGKme ShGKme merged commit 3463d0a into master Aug 31, 2023
16 checks passed
@ShGKme ShGKme deleted the perf/emoji-picker-mount branch August 31, 2023 13:00
@ShGKme
Copy link
Contributor Author

ShGKme commented Aug 31, 2023

/backport to stable7

@ShGKme ShGKme added 4. to release Ready to be released and/or waiting for tests to finish and removed backport-request 3. to review Waiting for reviews labels Aug 31, 2023
@Pytal Pytal mentioned this pull request Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement New feature or request feature: emoji picker Related to the emoji picker component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants