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

test(menu): maintain against memory leakage in menus #4056

Merged
merged 1 commit into from
Feb 20, 2024
Merged

Conversation

Westbrook
Copy link
Contributor

Description

While we weren't able to prove a specific leak before this change (likely addressed in v0.37.0 when we updated the Overlay API) this does do additional cache clearing for insurance purposes and places the work behind a used memory test to ensure that future changes will no regress on memory usage in this context.

Related issue(s)

How has this been tested?

  • Test case 1
    1. Go here
    2. This is predominately testing to ensure a baseline, so checkout the code in the change and see that it actually passed when run in the above job.

Types of changes

  • Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Checklist

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • If my change required a change to the documentation, I have updated the documentation in this pull request.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@Westbrook Westbrook requested a review from a team February 20, 2024 13:49
Copy link

github-actions bot commented Feb 20, 2024

Lighthouse scores

Category Latest (report) Main (report) Branch (report)
Performance 0.97 0.97 0.97
Accessibility 1 1 1
Best Practices 1 1 1
SEO 1 0.92 0.92
PWA 1 1 1
What is this?

Lighthouse scores comparing the documentation site built from the PR ("Branch") to that of the production documentation site ("Latest") and the build currently on main ("Main"). Higher scores are better, but note that the SEO scores on Netlify URLs are artifically constrained to 0.92.

Transfer Size

Category Latest Main Branch
Total 242.039 kB 229.581 kB 🏆 229.715 kB
Scripts 59.222 kB 53.996 kB 🏆 54.014 kB
Stylesheet 50.319 kB 43.774 kB 🏆 43.866 kB
Document 5.756 kB 5.099 kB 5.093 kB 🏆
Third Party 126.742 kB 126.712 kB 🏆 126.742 kB

Request Count

Category Latest Main Branch
Total 42 42 42
Scripts 34 34 34
Stylesheet 5 5 5
Document 1 1 1
Third Party 2 2 2

Copy link

github-actions bot commented Feb 20, 2024

Tachometer results

Chrome

action-menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 655 kB 173.42ms - 178.12ms - unsure 🔍
-2% - +2%
-3.12ms - +2.67ms
branch 647 kB 174.30ms - 177.69ms unsure 🔍
-2% - +2%
-2.67ms - +3.12ms
-

combobox permalink

Version Bytes Avg Time vs remote vs branch
npm latest 706 kB 39.16ms - 40.48ms - unsure 🔍
-1% - +3%
-0.22ms - +1.30ms
branch 698 kB 38.90ms - 39.67ms unsure 🔍
-3% - +1%
-1.30ms - +0.22ms
-

grid permalink

Version Bytes Avg Time vs remote vs branch
npm latest 463 kB 37.37ms - 37.89ms - slower ❌
1% - 3%
0.27ms - 1.02ms
branch 435 kB 36.72ms - 37.26ms faster ✔
1% - 3%
0.27ms - 1.02ms
-

menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 480 kB 241.78ms - 245.16ms - unsure 🔍
-1% - +1%
-3.62ms - +2.08ms
branch 471 kB 241.94ms - 246.53ms unsure 🔍
-1% - +1%
-2.08ms - +3.62ms
-

picker permalink

Version Bytes Avg Time vs remote vs branch
npm latest 518 kB 625.27ms - 633.86ms - unsure 🔍
-2% - +0%
-10.90ms - +2.70ms
branch 509 kB 628.39ms - 638.94ms unsure 🔍
-0% - +2%
-2.70ms - +10.90ms
-

split-button permalink

Version Bytes Avg Time vs remote vs branch
npm latest 721 kB 1858.17ms - 1860.95ms - unsure 🔍
-0% - +0%
-3.16ms - +0.93ms
branch 713 kB 1859.17ms - 1862.18ms unsure 🔍
-0% - +0%
-0.93ms - +3.16ms
-
Firefox

action-menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 655 kB 333.50ms - 343.06ms - unsure 🔍
-2% - +2%
-6.85ms - +6.29ms
branch 647 kB 334.05ms - 343.07ms unsure 🔍
-2% - +2%
-6.29ms - +6.85ms
-

combobox permalink

Version Bytes Avg Time vs remote vs branch
npm latest 706 kB 69.50ms - 75.46ms - slower ❌
6% - 15%
4.01ms - 10.07ms
branch 698 kB 64.94ms - 65.94ms faster ✔
6% - 13%
4.01ms - 10.07ms
-

grid permalink

Version Bytes Avg Time vs remote vs branch
npm latest 463 kB 78.70ms - 82.30ms - unsure 🔍
-2% - +5%
-1.56ms - +4.16ms
branch 435 kB 76.98ms - 81.42ms unsure 🔍
-5% - +2%
-4.16ms - +1.56ms
-

menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 480 kB 440.63ms - 450.01ms - unsure 🔍
-3% - +0%
-12.70ms - +2.14ms
branch 471 kB 444.85ms - 456.35ms unsure 🔍
-0% - +3%
-2.14ms - +12.70ms
-

picker permalink

Version Bytes Avg Time vs remote vs branch
npm latest 518 kB 1028.87ms - 1035.65ms - unsure 🔍
-1% - +1%
-8.68ms - +7.04ms
branch 509 kB 1025.99ms - 1040.17ms unsure 🔍
-1% - +1%
-7.04ms - +8.68ms
-

split-button permalink

Version Bytes Avg Time vs remote vs branch
npm latest 721 kB 1569.53ms - 1574.27ms - unsure 🔍
-0% - +0%
-1.61ms - +4.53ms
branch 713 kB 1568.49ms - 1572.39ms unsure 🔍
-0% - +0%
-4.53ms - +1.61ms
-

@hunterloftis hunterloftis self-assigned this Feb 20, 2024
@Westbrook Westbrook merged commit 122b36d into main Feb 20, 2024
48 checks passed
@Westbrook Westbrook deleted the menu-gc branch February 20, 2024 15:41
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.

[Memory-leak]: SWC Menu.ts removes the menu items on disconnect, but it doesn't clear out it's lookup set
2 participants