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

Add a few test for the hierarchySort function #22048

Closed
wants to merge 1 commit into from

Conversation

crazytonyli
Copy link
Contributor

Issue

The hierarchySort function is extremely slow. It only contains three statements, but every single one of them are O(n^2) functions.

func hierarchySort() -> [Element] {
return map {
$0.hasVisibleParent = !$1.containsPage(for: $0.parentID?.intValue)
return $0
}
.sort()
.hierachyIndexes()
}

Test in the app

The issue can be observed on Pages List. Due to an existing bug #21813, Pages List on a site that has lots of pages can't be displayed. If you want to see this issue in a real app, check out this branch, launch the app from Xcode to an iOS device, choose Field Guide (or any site that have 500+ pages), go to pages list, and scroll through the pages.

From my test, the issue is so bad that it's miserable to scroll though Field Guide pages. For a site with 600 pages, scrolling through the list would still gets jerky during fetching the pages.

This PR is not a fix

I'm planning to fix this issue, but this PR is not it. This PR adds some basic unit tests for this function and I'll work on addressing the issue separately.

Regression Notes

N/A

@crazytonyli crazytonyli requested a review from mokagio November 16, 2023 02:53
@crazytonyli crazytonyli self-assigned this Nov 16, 2023
@crazytonyli crazytonyli added this to the 23.8 milestone Nov 16, 2023
@wpmobilebot
Copy link
Contributor

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr22048-e149f6b
Version23.7
Bundle IDorg.wordpress.alpha
Commite149f6b
App Center BuildWPiOS - One-Offs #7853
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr22048-e149f6b
Version23.7
Bundle IDcom.jetpack.alpha
Commite149f6b
App Center Buildjetpack-installable-builds #6878
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

auto-merge was automatically disabled November 17, 2023 05:13

Pull request was closed

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.

2 participants