Cherry pick PR2285

Author: mitesh-patel-crm

packages/pwa-kit-runtime/CHANGELOG.md

@@ -1,4 +1,7 @@
 ## v3.9.1 (Mar 05, 2025)
+
+- Remove forced garbage collection on each invocation. Set `FORCE_GC=true` for the old behavior.
+
 ## v3.9.0 (Feb 18, 2025)
 - Fix stale service worker file that could cause requests to still use old Content-Security-Policy [#2191](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2191)
 - Support Node 22 [#2218](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2218)

packages/pwa-kit-runtime/src/ssr/server/build-remote-server.js

@@ -388,6 +388,8 @@ export const RemoteServerFactory = {
         const mixin = {
             options,
 
+            // Forcing a GC is no longer necessary, and will be
+            // skipped by default (unless FORCE_GC env-var is set).
             _collectGarbage() {
                 // Do global.gc in a separate 'then' handler so
                 // that all major variables are out of scope and
@@ -1044,12 +1046,15 @@ export const RemoteServerFactory = {
             context.callbackWaitsForEmptyEventLoop = false
 
             if (lambdaContainerReused) {
-                // DESKTOP-434 If this Lambda container is being reused,
-                // clean up memory now, so that we start with low usage.
-                // These regular GC calls take about 80-100 mS each, as opposed
-                // to forced GC calls, which occur randomly and can take several
-                // hundred mS.
-                app._collectGarbage()
+                const forceGarbageCollection = process.env.FORCE_GC
+                if (forceGarbageCollection && forceGarbageCollection.toLowerCase() === 'true') {
+                    // DESKTOP-434 If this Lambda container is being reused,
+                    // clean up memory now, so that we start with low usage.
+                    // These regular GC calls take about 80-100 mS each, as opposed
+                    // to forced GC calls, which occur randomly and can take several
+                    // hundred mS.
+                    app._collectGarbage()
+                }
                 app.sendMetric('LambdaReused')
             } else {
                 // This is the first use of this container, so set the

packages/pwa-kit-runtime/src/ssr/server/express.lambda.test.js

@@ -50,6 +50,48 @@ const httpsAgent = new https.Agent({
     rejectUnauthorized: false
 })
 
+function createServerWithGCSpy() {
+    const route = jest.fn((req, res) => {
+        res.send('<html/>')
+    })
+
+    const options = {
+        buildDir: testFixtures,
+        mobify: testPackageMobify,
+        sslFilePath: path.join(testFixtures, 'localhost.pem'),
+        quiet: true,
+        port: TEST_PORT,
+        fetchAgents: {
+            https: httpsAgent
+        }
+    }
+
+    const {handler, server, app} = RemoteServerFactory.createHandler(options, (app) => {
+        app.get('/*', route)
+    })
+
+    const collectGarbage = jest.spyOn(app, '_collectGarbage')
+    const sendMetric = jest.spyOn(app, 'sendMetric')
+    return {route, handler, collectGarbage, sendMetric, server}
+}
+
+function createApiGatewayEvent() {
+    // Set up a fake event and a fake context for the Lambda call
+    const event = createEvent('aws:apiGateway', {
+        path: '/',
+        body: undefined
+    })
+
+    if (event.queryStringParameters) {
+        delete event.queryStringParameters
+    }
+
+    const context = AWSMockContext({
+        functionName: 'SSRTest'
+    })
+    return {event, context}
+}
+
 describe('SSRServer Lambda integration', () => {
     let savedEnvironment
     let server
@@ -371,47 +413,40 @@ describe('SSRServer Lambda integration', () => {
         })
     })
 
-    test('Lambda reuse behaviour', () => {
-        const route = jest.fn((req, res) => {
-            res.send('<html/>')
-        })
-
-        const options = {
-            buildDir: testFixtures,
-            mobify: testPackageMobify,
-            sslFilePath: path.join(testFixtures, 'localhost.pem'),
-            quiet: true,
-            port: TEST_PORT,
-            fetchAgents: {
-                https: httpsAgent
-            }
-        }
-
-        const {
-            app,
-            handler,
-            server: srv
-        } = RemoteServerFactory.createHandler(options, (app) => {
-            app.get('/*', route)
-        })
-
-        const collectGarbage = jest.spyOn(app, '_collectGarbage')
-        const sendMetric = jest.spyOn(app, 'sendMetric')
-        server = srv
+    test('Lambda reuse -- Default Behavior', () => {
+        const {route, handler, collectGarbage, sendMetric, new_server} = createServerWithGCSpy()
+        const {event, context} = createApiGatewayEvent()
+        server = new_server
 
-        // Set up a fake event and a fake context for the Lambda call
-        const event = createEvent('aws:apiGateway', {
-            path: '/',
-            body: undefined
-        })
+        const call = (event) =>
+            new Promise((resolve) => handler(event, context, (err, response) => resolve(response)))
 
-        if (event.queryStringParameters) {
-            delete event.queryStringParameters
-        }
+        return Promise.resolve()
+            .then(() => call(event))
+            .then((response) => {
+                // First request - Lambda container created
+                expect(response.statusCode).toBe(200)
+                expect(collectGarbage.mock.calls).toHaveLength(0)
+                expect(route.mock.calls).toHaveLength(1)
+                expect(sendMetric).toHaveBeenCalledWith('LambdaCreated')
+                expect(sendMetric).not.toHaveBeenCalledWith('LambdaReused')
+            })
+            .then(() => call(event))
+            .then((response) => {
+                // Second call - Lambda container reused
+                expect(response.statusCode).toBe(200)
+                expect(collectGarbage.mock.calls).toHaveLength(0)
+                expect(route.mock.calls).toHaveLength(2)
+                expect(sendMetric).toHaveBeenCalledWith('LambdaCreated')
+                expect(sendMetric).toHaveBeenCalledWith('LambdaReused')
+            })
+    })
 
-        const context = AWSMockContext({
-            functionName: 'SSRTest'
-        })
+    test('Lambda reuse -- with Forced Garbage Collection Enabled', () => {
+        process.env.FORCE_GC = 'true'
+        const {event, context} = createApiGatewayEvent()
+        const {route, handler, collectGarbage, sendMetric, new_server} = createServerWithGCSpy()
+        server = new_server
 
         const call = (event) =>
             new Promise((resolve) => handler(event, context, (err, response) => resolve(response)))

packages/template-mrt-reference-app/app/ssr.js

@@ -124,7 +124,7 @@ const filterAndSortObjectKeys = (o, whitelist) =>
 /**
  * Return a JSON-serializable object with key diagnostic values from a request
  */
-const jsonFromRequest = (req) => {
+const jsonFromRequest = (req, additional_info) => {
     return {
         args: req.query,
         protocol: req.protocol,
@@ -136,7 +136,8 @@ const jsonFromRequest = (req) => {
         headers: sortObjectKeys(req.headers),
         ip: req.ip,
         env: filterAndSortObjectKeys(process.env, ENVS_TO_EXPOSE),
-        timestamp: new Date().toISOString()
+        timestamp: new Date().toISOString(),
+        ...(typeof additional_info === 'object' ? {additional_info} : {})
     }
 }
 
@@ -207,6 +208,98 @@ const responseHeadersTest = async (req, res) => {
     res.json(jsonFromRequest(req))
 }
 
+/**
+ * Express handler that allocates a lot of memory, and then removes
+ * a reference to the objects, such that they may be garbage collected.
+ */
+const memoryTest = async (req, res) => {
+    const memory_before = process.memoryUsage()
+    const test_count = req?.query?.count ? parseInt(req.query.count) : 1
+    const test_size = req?.query?.size ? parseInt(req.query.size) : 1024
+    const force_gc =
+        global?.gc && req?.query && (parseBoolean(req.query.forcegc) || parseBoolean(req.query.gc))
+
+    // allocate temporary memory blocks
+    const malloc_time_start = Date.now()
+    allocateMemory(test_count, test_size)
+    const malloc_time_ms = Date.now() - malloc_time_start
+
+    const gc_time_start = Date.now()
+    if (force_gc) {
+        global.gc()
+    }
+    const gc_time_ms = Date.now() - gc_time_start
+
+    const memory_after = process.memoryUsage()
+    const memory_delta = calculateNumericDeltas(memory_after, memory_before)
+    const factor = 10
+    const test_total_alloc_mb =
+        Math.round(((test_count * test_size) / 1024 / 1024) * factor) / factor
+    const additional_info = {
+        memory_end: memory_after,
+        memory_delta: memory_delta,
+        malloc_time: malloc_time_ms,
+        gc_time: gc_time_ms,
+        force_gc: force_gc,
+        test_count: test_count,
+        test_size: test_size,
+        test_total_alloc_mb: test_total_alloc_mb
+    }
+    res.json(jsonFromRequest(req, additional_info))
+}
+
+/**
+ * Allocate memory and lose the reference to it (so it can be cleaned up).
+ */
+function allocateMemory(test_count, test_size) {
+    const buffer = []
+    for (let index = 0; index < test_count; index++) {
+        buffer.push(Buffer.alloc(test_size, 0, 'ascii'))
+    }
+    return buffer.length
+}
+
+/**
+ * Calculate the numeric differences between two objects.
+ */
+function calculateNumericDeltas(obj1, obj2) {
+    const delta = {}
+    for (const key in obj1) {
+        if (
+            Object.prototype.hasOwnProperty.call(obj1, key) &&
+            Object.prototype.hasOwnProperty.call(obj2, key)
+        ) {
+            if (typeof obj1[key] === 'number' && typeof obj2[key] === 'number') {
+                delta[key] = obj1[key] - obj2[key]
+            }
+        }
+    }
+    return delta
+}
+
+/**
+ * Parse boolean value from string
+ */
+function parseBoolean(string_value) {
+    if (
+        string_value === null ||
+        string_value === undefined ||
+        typeof string_value !== 'string' ||
+        string_value.trim() === ''
+    ) {
+        return false
+    }
+    const lowerCaseValue = string_value.toLowerCase()
+    return (
+        lowerCaseValue === 'true' ||
+        lowerCaseValue === '1' ||
+        lowerCaseValue === 'on' ||
+        lowerCaseValue === 'enable' ||
+        lowerCaseValue === 'enabled' ||
+        lowerCaseValue === 'yes'
+    )
+}
+
 /**
  * Express handler that echos back a JSON response with
  * headers supplied in the request.
@@ -273,6 +366,7 @@ const {handler, app, server} = runtime.createHandler(options, (app) => {
     app.get('/tls', tlsVersionTest)
     app.get('/cache', cacheTest)
     app.get('/cache/:duration(\\d+)', cacheTest)
+    app.get('/memtest', memoryTest)
     app.get('/cookie', cookieTest)
     app.get('/headers', headerTest)
     app.get('/isolation', isolationTests)

packages/template-mrt-reference-app/app/ssr.test.js

@@ -59,7 +59,8 @@ describe('server', () => {
         ['/cache', 200, 'application/json; charset=utf-8'],
         ['/cookie', 200, 'application/json; charset=utf-8'],
         ['/set-response-headers', 200, 'application/json; charset=utf-8'],
-        ['/isolation', 200, 'application/json; charset=utf-8']
+        ['/isolation', 200, 'application/json; charset=utf-8'],
+        ['/memtest', 200, 'application/json; charset=utf-8']
     ])('Path %p should render correctly', (path, expectedStatus, expectedContentType) => {
         return request(app)
             .get(path)

packages/template-mrt-reference-app/jest.config.js

@@ -10,13 +10,18 @@ module.exports = {
     ...base,
     coverageThreshold: {
         global: {
-            branches: 64,
+            branches: 50,
             functions: 77,
             lines: 85,
             statements: 85
         }
     },
-    collectCoverageFrom: ['app/**'],
+    collectCoverageFrom: [
+        'app/**',
+        '!app/request-processor.js',
+        '!app/static/**',
+        '!app/*.json'
+    ],
     // Increase to: 6 x default timeout of 5 seconds
     ...(process.env.CI ? {testTimeout: 30000} : {})
 }