Stats: Improve scrolling performance #22847
Merged
Conversation
TopTotalsCell was calling addRows on every configuration of cell which in turn created and added a hierarchy of UIStackView-based views. Optimizing TopTotalsCell to only add rows once and then make manipulations on existing rows.
Contributor
|
| App Name | WordPress Alpha |
|
| Configuration | Release-Alpha | |
| Build Number | pr22847-465047a | |
| Version | 25.1 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | 465047a | |
| App Center Build | WPiOS - One-Offs #10211 |
Contributor
|
| App Name | Jetpack Alpha |
|
| Configuration | Release-Alpha | |
| Build Number | pr22847-465047a | |
| Version | 25.1 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | 465047a | |
| App Center Build | jetpack-installable-builds #9260 |
Contributor
Author
|
@guarani, do you still have capacity to review this? |
Contributor
|
I hope so, I'll try reviewing this week! |
kean
reviewed
Jun 20, 2024
WordPress/Classes/ViewRelated/Stats/Period Stats/SiteStatsPeriodTableViewController.swift
Show resolved
Hide resolved
WordPress/Classes/ViewRelated/Stats/Period Stats/Countries/CountriesCell.swift
Show resolved
Hide resolved
kean
reviewed
Jun 20, 2024
WordPress/Classes/ViewRelated/Stats/Extensions/UITableViewCell+Stats.swift
Outdated
Show resolved
Hide resolved
kean
approved these changes
Jun 20, 2024
These calls were added together with dynamic type support, however, they slow down layout process of the cell
Collaborator
4e409e3 to
bc88b72
Compare
jkmassel
added a commit
that referenced
this pull request
Jul 5, 2024
commit e0fd9fd Author: Gio Lodi <gio.lodi@automattic.com> Date: Wed Jul 3 15:08:20 2024 +1000 Explicitly set mac queue in Buildkite pipeline (#23402) commit 312df0f Author: Gio Lodi <gio.lodi@automattic.com> Date: Mon Jul 1 17:37:37 2024 +1000 Bump Fastlane `xcodebuild -showBuildSettings` interval (#23403) This is an attempt to avoid timeouts, mostly in CI. commit bae5a91 Author: WordPress Mobile Bot Account <mobile+wpmobilebot@automattic.com> Date: Thu Jun 27 09:08:50 2024 +1000 Merge 25.1 release finalization (#23391) * Fix announcement card keep showing up after tapping Done (#23384) * Update app translations – `Localizable.strings` * Update WordPress metadata translations * Update Jetpack metadata translations * Bump version number --------- Co-authored-by: David Christiandy <1299411+dvdchr@users.noreply.github.com> commit c508e4f Author: Alex Grebenyuk <alex.grebenyuk@automattic.com> Date: Tue Jun 25 23:47:11 2024 -0400 Remove AlamofireNetworkActivityIndicator (#23385) commit 7bc34eb Author: kean <grebenyuk.alexander@gmail.com> Date: Tue Jun 25 11:00:32 2024 -0400 Remove deprecated in JetpackBrandingVisibility.enabled commit 9c8ff9a Author: kean <grebenyuk.alexander@gmail.com> Date: Tue Jun 25 10:58:55 2024 -0400 Fix more warnings commit 55e0cd4 Author: kean <grebenyuk.alexander@gmail.com> Date: Tue Jun 25 10:53:34 2024 -0400 Fix warnings in CachedAsyncImage commit 1f78c63 Author: kean <grebenyuk.alexander@gmail.com> Date: Tue Jun 25 10:51:42 2024 -0400 Fix warnings in MemoryCache commit 9d86b6b Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Tue Jun 25 09:06:16 2024 +0200 Support editing media metadata via XML-RPC #809 (#23316) * Support media metadata editing for XML-RPC connected self-hosted sites - Updated WordPressKit supports editing title, description, and caption of the media via XML-RPC - XML-RPC API doesn't support editing alt-text * Support editing media metadata via XMLRPC in MediaService Media is a type of a post therefore "wp.editPost" can be used to edit media metadata. Note that alternative text cannot be edited due to lack of XML-RPC support https://core.trac.wordpress.org/ticket/58582 * Update RELEASE-NOTES.txt commit d8a1348 Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Tue Jun 25 09:05:13 2024 +0200 Add unique identifier to file downloads rows (#23310) File Downloads data can be identical which can result in a rare duplicate diffable data source identifiers crash. Pass a unique identifier to ensure that each file downloads row is treated as unique. commit 09137d3 Merge: fc50d1a c88e54a Author: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon Jun 24 14:16:17 2024 -0600 Reduce App Size – Assets (#23381) commit fc50d1a Merge: 2270784 32a3251 Author: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon Jun 24 14:15:55 2024 -0600 Remove Stories and Kanvas dependency (#23382) commit 2270784 Merge: 2ee755c f76856a Author: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon Jun 24 13:50:40 2024 -0600 Update rubocop.yml (#23383) commit f76856a Author: kean <grebenyuk.alexander@gmail.com> Date: Mon Jun 24 15:27:29 2024 -0400 Update rubocop.yml commit 32a3251 Author: kean <grebenyuk.alexander@gmail.com> Date: Mon Jun 24 13:52:46 2024 -0400 Remove Kanvas pod commit c93087b Author: kean <grebenyuk.alexander@gmail.com> Date: Mon Jun 24 13:30:10 2024 -0400 Remove StoriesIntroViewController commit e64e98d Author: kean <grebenyuk.alexander@gmail.com> Date: Mon Jun 24 13:25:40 2024 -0400 Replace remaining Kanvas usages commit 3ec3d0d Author: kean <grebenyuk.alexander@gmail.com> Date: Mon Jun 24 12:26:35 2024 -0400 Remove StoryEditor commit 4b66f07 Author: kean <grebenyuk.alexander@gmail.com> Date: Mon Jun 24 12:17:06 2024 -0400 Remove Kanvas related code commit 4812e5b Author: kean <grebenyuk.alexander@gmail.com> Date: Mon Jun 24 12:13:48 2024 -0400 Remove remaining Kanvas related code commit f7743b6 Author: kean <grebenyuk.alexander@gmail.com> Date: Mon Jun 24 12:11:30 2024 -0400 Remove custom fonts used by Kanvas commit c88e54a Author: kean <grebenyuk.alexander@gmail.com> Date: Mon Jun 24 10:44:12 2024 -0400 Replace JPBackground with tiny-fied icons commit a9d80c8 Author: kean <grebenyuk.alexander@gmail.com> Date: Mon Jun 24 10:33:29 2024 -0400 Reduce rppreview size commit ac82126 Author: kean <grebenyuk.alexander@gmail.com> Date: Mon Jun 24 10:27:29 2024 -0400 Remove unused site creation icons commit 5b8a5a3 Author: kean <grebenyuk.alexander@gmail.com> Date: Mon Jun 24 10:21:00 2024 -0400 Remove Stories related files commit 6f98ffd Author: kean <grebenyuk.alexander@gmail.com> Date: Mon Jun 24 10:19:06 2024 -0400 Move JPBackground to AppImage specific to the Jetpack app commit dd78ad7 Author: kean <grebenyuk.alexander@gmail.com> Date: Mon Jun 24 10:09:20 2024 -0400 JPBackground as png commit 2ee755c Merge: 81bc1c5 3835956 Author: Alex Grebenyuk <alex.grebenyuk@automattic.com> Date: Fri Jun 21 10:13:38 2024 -0400 Fix rare crash in GutenbergWebViewController (#23379) commit 81bc1c5 Merge: 6dc78ec 465047a Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Fri Jun 21 13:30:39 2024 +0300 Stats: Improve scrolling performance (#22847) commit 465047a Merge: bc88b72 6dc78ec Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Fri Jun 21 11:00:43 2024 +0300 Merge branch 'trunk' into task/22721-stats-traffic-improve-scrolling-performance commit bc88b72 Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Fri Jun 21 10:57:00 2024 +0300 Put analyticsTracker back since it's used by JetpackBanner commit 1b8bc1c Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Fri Jun 21 10:52:21 2024 +0300 Create StatsRowsCell with default child stack view rows and ability to configure more commit 89fd816 Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Fri Jun 21 10:28:28 2024 +0300 Make maximum content size category smaller for stats cell subtitles commit 51992ab Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Fri Jun 21 10:28:01 2024 +0300 Remove force layout calls when setting subtitle visibility These calls were added together with dynamic type support, however, they slow down layout process of the cell commit 3835956 Author: kean <grebenyuk.alexander@gmail.com> Date: Thu Jun 20 10:27:02 2024 -0400 Update release notes commit 26bd9b3 Author: kean <grebenyuk.alexander@gmail.com> Date: Thu Jun 20 10:23:16 2024 -0400 Fix rare crash in GutenbergWebViewController commit 7684a35 Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Thu Jun 20 10:48:25 2024 +0300 Move additional checks for adding default rows into the extension commit 25bf46d Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Wed Jun 19 13:47:26 2024 +0300 Update RELEASE-NOTES commit 8884f81 Merge: 2674401 1c05916 Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Wed Jun 19 13:35:52 2024 +0300 Merge branch 'trunk' into task/22721-stats-traffic-improve-scrolling-performance commit 2674401 Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Thu Apr 11 15:57:07 2024 +0300 Update CountriesCell to use setNeedsLayout for more efficiency commit eb7c142 Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Thu Apr 11 15:45:56 2024 +0300 Update TopTotalsCell to use setNeedsLayout for more efficiency commit d0af997 Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Wed Apr 10 11:40:38 2024 +0300 Update RELEASE-NOTES.txt commit 8fb2eb4 Merge: 9252c6f d79aa28 Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Wed Apr 10 10:07:24 2024 +0300 Merge branch 'trunk' into task/22721-stats-traffic-improve-scrolling-performance commit 9252c6f Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Mon Mar 18 17:05:19 2024 +0200 Do not track StatsTraffic tableView scrolling commit 9880cb7 Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Mon Mar 18 17:05:02 2024 +0200 Optimize CountriesCell to add rows only when the cell loads commit 98d5b77 Author: Povilas Staskus <povilas.staskus@automattic.com> Date: Mon Mar 18 16:45:44 2024 +0200 Optimize TopTotalsCell to add rows only when the cell loads TopTotalsCell was calling addRows on every configuration of cell which in turn created and added a hierarchy of UIStackView-based views. Optimizing TopTotalsCell to only add rows once and then make manipulations on existing rows.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Fixes #22721
Majority of Stats Traffic / Stats Period scrolling hitches happen due to inefficient
TopTotalsCellload, mainly calls toaddRowswhich creates and adds multiple layers ofUIStackViewsevery time the cell is configured.Investigation: pc8eDl-1pe-p2
Solution
UIStackViewonly once when the table view loads. When other cells reuse the same views, only configure or hide existing views.setNeedsLayoutinstead oflayoutIfNeededThis is only the first iteration of possible improvements. I didn't achieve a full smooth experience and animation hitches still happen after the first load. However, the improvements are measurable and visible, especially when testing on a debugger-connected iPhone.
Performance Comparisons
I decided to compare the performance of cell configuration methods by measuring how much time it takes to fully configure the cell. When testing, I use a high-traffic site and scroll up and down three times.
TopTotalsCell
iPhone 14 Pro
CountriesCell
iPhone 14 Pro
All Stats Cells Averages
The improvement of the configuration of all the Stats cell averages falls somewhere in between of two numbers.
iPhone 14 Pro
Hitches Comparisons
Before
After
Remaining Hitches
The majority of those remaining hitches are caused by issues that couldn't be identified or sometimes identified simply as
mainusing Time Profiler:To test:
trunkwith a debugger connectedtask/22721-stats-traffic-improve-scrolling-performancewith a debugger connectedRegression Notes
TopTotalsCell is only used within Stats, doesn't affect any other areas
Manual testing, and performance measurement
None
PR submission checklist:
RELEASE-NOTES.txtif necessary.Testing checklist: