Fix memory leak in useIntersection #20407

tvler · 2020-12-22T22:39:23Z

This pull request adds an elements.delete operation to the useIntersection's cleanup function: unobserve.

Without this delete operation, next.js holds onto an unreachable reference of every observed element indefinitely (automatically every Link and Image is observed, so that means every rendered Link and Image element adds to the leak). I found this memory leak when building out an infinite feed in next.js with thousands of Link elements.

The final code block of the unobserve function body:

  // Destroy observer when there's nothing left to watch:
  if (elements.size === 0) {
    observer.disconnect()
    observers.delete(id)
  }

Is effectively unreachable without this delete operation, as the elements map will never decrease in size as it is currently. This means that there will always be at least one IntersectionObserver instance in memory if useIntersection has been used once, regardless of if there are currently any components still using the hook.

tvler · 2020-12-22T22:47:49Z

Looks like it was added here #18904 cc @Timer

tvler · 2020-12-22T23:34:27Z

The build errors seem pretty unrelated to this code – do I need to rebase my branch or do anything on my end to make this green?

ijjk · 2020-12-27T00:07:57Z

Stats from current PR

Default Server Mode (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
buildDuration	10.9s	10.8s	-147ms
nodeModulesSize	82.6 MB	82.6 MB	⚠️ +90 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.197	2.201	0
/ avg req/sec	1137.7	1136.1	⚠️ -1.6
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.576	1.578	0
/error-in-render avg req/sec	1586.63	1584.33	⚠️ -2.3

Client Bundles (main, webpack, commons)

	vercel/next.js canary	tvler/next.js link-memory	Change
677f882d2ed8..5e70.js gzip	12.8 kB	12.8 kB	✓
framework.HASH.js gzip	39 kB	39 kB	✓
main-e0d2962..b163.js gzip	6.56 kB	6.56 kB	✓
webpack-95c2..e870.js gzip	751 B	751 B	✓
Overall change	59 kB	59 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	tvler/next.js link-memory	Change
polyfills-d3..23f6.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
_app-0d19cb6..5497.js gzip	1.28 kB	1.28 kB	✓
_error-85785..a9f3.js gzip	3.44 kB	3.44 kB	✓
hooks-42456f..0c06.js gzip	887 B	887 B	✓
index-8081ce..e44f.js gzip	227 B	227 B	✓
link-0ab9f83..fa00.js gzip	1.61 kB	1.61 kB	⚠️ +3 B
routerDirect..c3d8.js gzip	303 B	303 B	✓
withRouter-0..a68e.js gzip	302 B	302 B	✓
Overall change	8.05 kB	8.05 kB	⚠️ +3 B

Client Build Manifests Overall decrease ✓

	vercel/next.js canary	tvler/next.js link-memory	Change
_buildManifest.js gzip	323 B	322 B	-1 B
Overall change	323 B	322 B	-1 B

Rendered Page Sizes

	vercel/next.js canary	tvler/next.js link-memory	Change
index.html gzip	612 B	612 B	✓
link.html gzip	620 B	620 B	✓
withRouter.html gzip	607 B	607 B	✓
Overall change	1.84 kB	1.84 kB	✓

Diffs

Diff for _buildManifest.js

@@ -7,7 +7,7 @@ self.__BUILD_MANIFEST = {
   "/hooks": [
     "static\u002Fchunks\u002Fpages\u002Fhooks-bdd2cad07648acf22380.js"
   ],
-  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-00f0f71a76f57326f2aa.js"],
+  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-d3b8250e23ab1645868e.js"],
   "/routerDirect": [
     "static\u002Fchunks\u002Fpages\u002FrouterDirect-6f408582ce776dfa74d5.js"
   ],

Diff for link-00f0f71..57326f2aa.js

@@ -376,6 +376,7 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
         elements.set(element, callback);
         observer.observe(element);
         return function unobserve() {
+          elements["delete"](element);
           observer.unobserve(element); // Destroy observer when there's nothing left to watch:
 
           if (elements.size === 0) {

Diff for link.html

@@ -32,7 +32,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/pages/link-00f0f71a76f57326f2aa.js"
+      href="/_next/static/chunks/pages/link-d3b8250e23ab1645868e.js"
       as="script"
     />
   </head>
@@ -78,7 +78,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/pages/link-00f0f71a76f57326f2aa.js"
+      src="/_next/static/chunks/pages/link-d3b8250e23ab1645868e.js"
       async=""
     ></script>
     <script src="/_next/static/BUILD_ID/_buildManifest.js" async=""></script>

Serverless Mode (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
buildDuration	12.7s	12.8s	⚠️ +118ms
nodeModulesSize	82.6 MB	82.6 MB	⚠️ +90 B

Client Bundles (main, webpack, commons)

	vercel/next.js canary	tvler/next.js link-memory	Change
677f882d2ed8..5e70.js gzip	12.8 kB	12.8 kB	✓
framework.HASH.js gzip	39 kB	39 kB	✓
main-e0d2962..b163.js gzip	6.56 kB	6.56 kB	✓
webpack-95c2..e870.js gzip	751 B	751 B	✓
Overall change	59 kB	59 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	tvler/next.js link-memory	Change
polyfills-d3..23f6.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
_app-0d19cb6..5497.js gzip	1.28 kB	1.28 kB	✓
_error-85785..a9f3.js gzip	3.44 kB	3.44 kB	✓
hooks-42456f..0c06.js gzip	887 B	887 B	✓
index-8081ce..e44f.js gzip	227 B	227 B	✓
link-0ab9f83..fa00.js gzip	1.61 kB	N/A	N/A
routerDirect..c3d8.js gzip	303 B	303 B	✓
withRouter-0..a68e.js gzip	302 B	302 B	✓
link-3d5eee0..b692.js gzip	N/A	1.61 kB	N/A
Overall change	8.05 kB	8.05 kB	⚠️ +3 B

Client Build Manifests Overall decrease ✓

	vercel/next.js canary	tvler/next.js link-memory	Change
_buildManifest.js gzip	323 B	322 B	-1 B
Overall change	323 B	322 B	-1 B

Serverless bundles Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
_error.js	1 MB	1 MB	✓
404.html	2.67 kB	2.67 kB	✓
hooks.html	1.92 kB	1.92 kB	✓
index.js	1 MB	1 MB	✓
link.js	1.06 MB	1.06 MB	⚠️ +30 B
routerDirect.js	1.05 MB	1.05 MB	✓
withRouter.js	1.05 MB	1.05 MB	✓
Overall change	5.16 MB	5.16 MB	⚠️ +30 B

Commit: 0f107e5

ijjk · 2020-12-28T18:40:44Z

Stats from current PR

Default Server Mode (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
buildDuration	9s	9.1s	⚠️ +82ms
nodeModulesSize	82.6 MB	82.6 MB	⚠️ +90 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	tvler/next.js link-memory	Change
/ failed reqs	0	0	✓
/ total time (seconds)	1.789	1.754	-0.03
/ avg req/sec	1397.25	1425.57	+28.32
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.142	1.106	-0.04
/error-in-render avg req/sec	2189.75	2260.76	+71.01

Client Bundles (main, webpack, commons)

	vercel/next.js canary	tvler/next.js link-memory	Change
677f882d2ed8..1e91.js gzip	12.8 kB	12.8 kB	✓
framework.HASH.js gzip	39 kB	39 kB	✓
main-e0d2962..b163.js gzip	6.56 kB	6.56 kB	✓
webpack-95c2..e870.js gzip	751 B	751 B	✓
Overall change	59 kB	59 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	tvler/next.js link-memory	Change
polyfills-d3..23f6.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
_app-0d19cb6..5497.js gzip	1.28 kB	1.28 kB	✓
_error-85785..a9f3.js gzip	3.44 kB	3.44 kB	✓
hooks-42456f..0c06.js gzip	887 B	887 B	✓
index-8081ce..e44f.js gzip	227 B	227 B	✓
link-0ab9f83..fa00.js gzip	1.61 kB	1.61 kB	⚠️ +3 B
routerDirect..c3d8.js gzip	303 B	303 B	✓
withRouter-0..a68e.js gzip	302 B	302 B	✓
Overall change	8.05 kB	8.05 kB	⚠️ +3 B

Client Build Manifests Overall decrease ✓

	vercel/next.js canary	tvler/next.js link-memory	Change
_buildManifest.js gzip	323 B	322 B	-1 B
Overall change	323 B	322 B	-1 B

Rendered Page Sizes

	vercel/next.js canary	tvler/next.js link-memory	Change
index.html gzip	615 B	615 B	✓
link.html gzip	621 B	621 B	✓
withRouter.html gzip	609 B	609 B	✓
Overall change	1.84 kB	1.84 kB	✓

Diffs

Diff for _buildManifest.js

@@ -7,7 +7,7 @@ self.__BUILD_MANIFEST = {
   "/hooks": [
     "static\u002Fchunks\u002Fpages\u002Fhooks-bdd2cad07648acf22380.js"
   ],
-  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-00f0f71a76f57326f2aa.js"],
+  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-d3b8250e23ab1645868e.js"],
   "/routerDirect": [
     "static\u002Fchunks\u002Fpages\u002FrouterDirect-6f408582ce776dfa74d5.js"
   ],

Diff for link-00f0f71..57326f2aa.js

@@ -376,6 +376,7 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
         elements.set(element, callback);
         observer.observe(element);
         return function unobserve() {
+          elements["delete"](element);
           observer.unobserve(element); // Destroy observer when there's nothing left to watch:
 
           if (elements.size === 0) {

Diff for link.html

@@ -32,7 +32,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/pages/link-00f0f71a76f57326f2aa.js"
+      href="/_next/static/chunks/pages/link-d3b8250e23ab1645868e.js"
       as="script"
     />
   </head>
@@ -78,7 +78,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/pages/link-00f0f71a76f57326f2aa.js"
+      src="/_next/static/chunks/pages/link-d3b8250e23ab1645868e.js"
       async=""
     ></script>
     <script src="/_next/static/BUILD_ID/_buildManifest.js" async=""></script>

Serverless Mode (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
buildDuration	10.9s	10.4s	-492ms
nodeModulesSize	82.6 MB	82.6 MB	⚠️ +90 B

Client Bundles (main, webpack, commons)

	vercel/next.js canary	tvler/next.js link-memory	Change
677f882d2ed8..1e91.js gzip	12.8 kB	12.8 kB	✓
framework.HASH.js gzip	39 kB	39 kB	✓
main-e0d2962..b163.js gzip	6.56 kB	6.56 kB	✓
webpack-95c2..e870.js gzip	751 B	751 B	✓
Overall change	59 kB	59 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	tvler/next.js link-memory	Change
polyfills-d3..23f6.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
_app-0d19cb6..5497.js gzip	1.28 kB	1.28 kB	✓
_error-85785..a9f3.js gzip	3.44 kB	3.44 kB	✓
hooks-42456f..0c06.js gzip	887 B	887 B	✓
index-8081ce..e44f.js gzip	227 B	227 B	✓
link-0ab9f83..fa00.js gzip	1.61 kB	N/A	N/A
routerDirect..c3d8.js gzip	303 B	303 B	✓
withRouter-0..a68e.js gzip	302 B	302 B	✓
link-3d5eee0..b692.js gzip	N/A	1.61 kB	N/A
Overall change	8.05 kB	8.05 kB	⚠️ +3 B

Client Build Manifests Overall decrease ✓

	vercel/next.js canary	tvler/next.js link-memory	Change
_buildManifest.js gzip	323 B	322 B	-1 B
Overall change	323 B	322 B	-1 B

Serverless bundles Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
_error.js	1 MB	1 MB	✓
404.html	2.67 kB	2.67 kB	✓
hooks.html	1.92 kB	1.92 kB	✓
index.js	1 MB	1 MB	✓
link.js	1.06 MB	1.06 MB	⚠️ +30 B
routerDirect.js	1.05 MB	1.05 MB	✓
withRouter.js	1.05 MB	1.05 MB	✓
Overall change	5.16 MB	5.16 MB	⚠️ +30 B

Commit: f988876

ijjk · 2020-12-28T19:02:07Z

Failing test suites

Commit: f988876

test/integration/i18n-support-base-path/test/index.test.js

i18n Support basePath > serverless mode > should navigate through history with query correctly

Expand output

● i18n Support basePath › serverless mode › should navigate through history with query correctly

thrown: "Exceeded timeout of 120000 ms for a test.
Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

  28 | 
  29 | export function runTests(ctx) {
> 30 |   it('should navigate through history with query correctly', async () => {
     |   ^
  31 |     const browser = await webdriver(ctx.appPort, `${ctx.basePath || '/'}`)
  32 | 
  33 |     await browser.eval(`(function() {

  at runTests (integration/i18n-support/test/shared.js:30:3)
  at integration/i18n-support-base-path/test/index.test.js:191:5
  at integration/i18n-support-base-path/test/index.test.js:77:3
  at Object.<anonymous> (integration/i18n-support-base-path/test/index.test.js:24:1)

ijjk · 2020-12-30T19:11:05Z

Stats from current PR

Default Server Mode (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
buildDuration	10.2s	10.3s	⚠️ +192ms
nodeModulesSize	83 MB	83 MB	⚠️ +90 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.066	2.097	⚠️ +0.03
/ avg req/sec	1210.19	1192.42	⚠️ -17.77
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.28	1.281	0
/error-in-render avg req/sec	1953.36	1951.45	⚠️ -1.91

Client Bundles (main, webpack, commons)

	vercel/next.js canary	tvler/next.js link-memory	Change
677f882d2ed8..769b.js gzip	12.8 kB	12.8 kB	✓
framework.HASH.js gzip	39 kB	39 kB	✓
main-a6ce971..96b9.js gzip	6.59 kB	6.59 kB	✓
webpack-7193..1446.js gzip	751 B	751 B	✓
Overall change	59.1 kB	59.1 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	tvler/next.js link-memory	Change
polyfills-67..b7d1.js gzip	31.2 kB	31.2 kB	✓
Overall change	31.2 kB	31.2 kB	✓

Client Pages Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
_app-6220e08..9a40.js gzip	1.28 kB	1.28 kB	✓
_error-4b0b5..2c91.js gzip	3.44 kB	3.44 kB	✓
hooks-5f309a..7282.js gzip	887 B	887 B	✓
index-57f580..c562.js gzip	227 B	227 B	✓
link-21c7af4..bddc.js gzip	1.6 kB	1.61 kB	⚠️ +4 B
routerDirect..bd82.js gzip	303 B	303 B	✓
withRouter-2..e384.js gzip	302 B	302 B	✓
Overall change	8.04 kB	8.05 kB	⚠️ +4 B

Client Build Manifests Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
_buildManifest.js gzip	320 B	322 B	⚠️ +2 B
Overall change	320 B	322 B	⚠️ +2 B

Rendered Page Sizes Overall decrease ✓

	vercel/next.js canary	tvler/next.js link-memory	Change
index.html gzip	616 B	616 B	✓
link.html gzip	622 B	621 B	-1 B
withRouter.html gzip	609 B	609 B	✓
Overall change	1.85 kB	1.85 kB	-1 B

Diffs

Diff for _buildManifest.js

@@ -7,7 +7,7 @@ self.__BUILD_MANIFEST = {
   "/hooks": [
     "static\u002Fchunks\u002Fpages\u002Fhooks-61cb7b66e376b851f63a.js"
   ],
-  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-42154b79cc830a1016aa.js"],
+  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-93d772b087df6a15801f.js"],
   "/routerDirect": [
     "static\u002Fchunks\u002Fpages\u002FrouterDirect-a5bdcfc87579b1d2776d.js"
   ],

Diff for link-42154b7..30a1016aa.js

@@ -377,6 +377,7 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
         elements.set(element, callback);
         observer.observe(element);
         return function unobserve() {
+          elements["delete"](element);
           observer.unobserve(element); // Destroy observer when there's nothing left to watch:
 
           if (elements.size === 0) {

Diff for link.html

@@ -32,7 +32,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/pages/link-42154b79cc830a1016aa.js"
+      href="/_next/static/chunks/pages/link-93d772b087df6a15801f.js"
       as="script"
     />
   </head>
@@ -78,7 +78,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/pages/link-42154b79cc830a1016aa.js"
+      src="/_next/static/chunks/pages/link-93d772b087df6a15801f.js"
       async=""
     ></script>
     <script src="/_next/static/BUILD_ID/_buildManifest.js" async=""></script>

Serverless Mode (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
buildDuration	12.4s	12.3s	-51ms
nodeModulesSize	83 MB	83 MB	⚠️ +90 B

Client Bundles (main, webpack, commons)

	vercel/next.js canary	tvler/next.js link-memory	Change
677f882d2ed8..769b.js gzip	12.8 kB	12.8 kB	✓
framework.HASH.js gzip	39 kB	39 kB	✓
main-a6ce971..96b9.js gzip	6.59 kB	6.59 kB	✓
webpack-7193..1446.js gzip	751 B	751 B	✓
Overall change	59.1 kB	59.1 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	tvler/next.js link-memory	Change
polyfills-67..b7d1.js gzip	31.2 kB	31.2 kB	✓
Overall change	31.2 kB	31.2 kB	✓

Client Pages Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
_app-6220e08..9a40.js gzip	1.28 kB	1.28 kB	✓
_error-4b0b5..2c91.js gzip	3.44 kB	3.44 kB	✓
hooks-5f309a..7282.js gzip	887 B	887 B	✓
index-57f580..c562.js gzip	227 B	227 B	✓
link-21c7af4..bddc.js gzip	1.6 kB	N/A	N/A
routerDirect..bd82.js gzip	303 B	303 B	✓
withRouter-2..e384.js gzip	302 B	302 B	✓
link-1a1f628..eeb5.js gzip	N/A	1.61 kB	N/A
Overall change	8.04 kB	8.05 kB	⚠️ +4 B

Client Build Manifests Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
_buildManifest.js gzip	320 B	322 B	⚠️ +2 B
Overall change	320 B	322 B	⚠️ +2 B

Serverless bundles Overall increase ⚠️

	vercel/next.js canary	tvler/next.js link-memory	Change
_error.js	1 MB	1 MB	✓
404.html	2.67 kB	2.67 kB	✓
hooks.html	1.92 kB	1.92 kB	✓
index.js	1 MB	1 MB	✓
link.js	1.06 MB	1.06 MB	⚠️ +30 B
routerDirect.js	1.05 MB	1.05 MB	✓
withRouter.js	1.05 MB	1.05 MB	✓
Overall change	5.17 MB	5.17 MB	⚠️ +30 B

Commit: 7a348d2

Timer

Thanks!

fix memory leak

b82a875

tvler requested review from ijjk, lfades, Timer and timneutkens as code owners December 22, 2020 22:39

ijjk added the type: next label Dec 22, 2020

vercel bot temporarily deployed to Preview December 22, 2020 22:39 Inactive

Merge branch 'canary' into link-memory

ae87df1

vercel bot temporarily deployed to Preview December 22, 2020 22:42 Inactive

Merge branch 'canary' into link-memory

0f107e5

vercel bot temporarily deployed to Preview December 26, 2020 23:58 Inactive

Merge branch 'canary' into link-memory

f988876

vercel bot temporarily deployed to Preview December 28, 2020 18:32 Inactive

Merge branch 'canary' into link-memory

7a348d2

Timer added this to the iteration 15 milestone Dec 30, 2020

vercel bot temporarily deployed to Preview December 30, 2020 19:02 Inactive

Timer approved these changes Dec 30, 2020

View reviewed changes

kodiakhq bot merged commit ec430ea into vercel:canary Dec 30, 2020

kaykdm mentioned this pull request Jan 13, 2021

Link memory leak #21028

Closed

vercel locked as resolved and limited conversation to collaborators Jan 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix memory leak in useIntersection #20407

Fix memory leak in useIntersection #20407

tvler commented Dec 22, 2020

tvler commented Dec 22, 2020

tvler commented Dec 22, 2020

ijjk commented Dec 27, 2020

Diffs

ijjk commented Dec 28, 2020

Diffs

ijjk commented Dec 28, 2020

ijjk commented Dec 30, 2020

Diffs

Timer left a comment

Fix memory leak in useIntersection #20407

Fix memory leak in useIntersection #20407

Conversation

tvler commented Dec 22, 2020

tvler commented Dec 22, 2020

tvler commented Dec 22, 2020

ijjk commented Dec 27, 2020

Stats from current PR

Diffs

ijjk commented Dec 28, 2020

Stats from current PR

Diffs

ijjk commented Dec 28, 2020

Failing test suites

ijjk commented Dec 30, 2020

Stats from current PR

Diffs

Timer left a comment

Choose a reason for hiding this comment