🐛 [RUM-4436] slidingSessionWindow returns selector instead of node to allow garbage collection of unused nodes after view end

[thomas-lebeau]

Motivation

trackCumulativeLayoutShift keep track of the LCP target Element using a sliding window of layout shift events but this window is cleared 5 minutes after the view ends.

Within these 5 minutes, many new view events might be started meaning an increase in memory consumption du to the reference to the LCP target Element kept and preventing garbage collection of these elements (and potentially large amount of descendent)

Changes

as a sliding window cannot last more that 5 seconds, uses a timeout to clear the window (and the target element reference) every 5 seconds so when the GC kicks in they can be freed if necessary

Testing

• [x] Local
• [ ] Staging
• [x] Unit
• [ ] End to end

---

I have gone over the contributing documentation.

[cit-pr-commenter[bot]]

Bundles Sizes Evolution

📦 Bundle Name	Base Size	Local Size	𝚫	𝚫%	Status
Rum	157.60 KiB	157.55 KiB	-44 B	-0.03%	✅
Logs	56.29 KiB	56.29 KiB	0 B	0.00%	✅
Rum Slim	104.09 KiB	104.04 KiB	-48 B	-0.05%	✅
Worker	25.21 KiB	25.21 KiB	0 B	0.00%	✅

🚀 CPU Performance

Action Name	Base Average Cpu Time (ms)	Local Average Cpu Time (ms)	𝚫
adderror	0.030	0.038	0.008
addaction	0.014	N/A	N/A
logmessage	0.005	0.005	0.000
startview	0.804	0.955	0.151
startstopsessionreplayrecording	0.762	0.836	0.074
addtiming	0.001	0.001	0.000
addglobalcontext	0.001	0.001	0.000

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 93.28%. Comparing base (b0d638c) to head (ca2802a). Report is 9 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2749      +/-   ##
==========================================
+ Coverage   93.27%   93.28%   +0.01%     
==========================================
  Files         241      241              
  Lines        7034     7032       -2     
  Branches     1554     1555       +1     
==========================================
- Hits         6561     6560       -1     
+ Misses        473      472       -1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[thomas-lebeau]

~@BenoitZugmeyer I think this is not possible. The problem is that we need to calculate the selector of the largest layout shift within the window and check if it's still connected. Because of this we need to keep a reference to target of the largest layout shift for as long as the window is open.~

Edit: As discussed during our 1:1, the isConnected check is only relevant to calculate the selector (Because performance event are async, it's possible that the node is disconnected at the time the event is handled). With the changes in this PR, it is now possible to report a selector instead of undefined for nodes that are responsible for the largest shift but are disconnected before the folowing shift within the same window.

[thomas-lebeau]

/to-staging

[dd-devflow[bot]]

:steam_locomotive: Branch Integration: starting soon, merge in < 9m

Commit 4ebadc8b01 will soon be integrated into staging-20.

This build is going to start soon! (estimated merge in less than 9m)

Use /to-staging -c to cancel this operation!

[dd-devflow[bot]]

:steam_locomotive: Branch Integration: This commit was successfully integrated

Commit 4ebadc8b01 has been merged into staging-20 in merge commit 173bf4e0d7.

Check out the triggered pipeline on Gitlab :fox_face:

[amortemousque]

🔨 warning: ‏With this implementation we are computing the CSS selector lot more often:

• For each window even if it's not a maxClsValue
• For each shift entry if the value is superior to the precedent Since layout shift can be frequently triggered by some UI elements, it can have an impact on the processing.