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

fix suggestions performance #1008

Merged
merged 1 commit into from
Sep 27, 2024
Merged

Conversation

brindy
Copy link
Contributor

@brindy brindy commented Sep 27, 2024

Required:

Task/Issue URL: https://app.asana.com/0/1201048563534612/1208413716679959/f
iOS PR: duckduckgo/iOS#3405
macOS PR: duckduckgo/macos-browser#3353
What kind of version bump will this require?: Major/Minor/Patch

Optional:

Tech Design URL:
CC:

Description:
Fix suggestions performance issue by just looking at the first 100 sorted local results.

Steps to test this PR:

  1. Run the app on a local large data source (history entries > 1000 entries ideally)
  2. Use autocomplete and check that it doesn't time out for a broad range of matches

@brindy brindy self-assigned this Sep 27, 2024
@brindy brindy marked this pull request as ready for review September 27, 2024 13:02
@brindy brindy requested a review from tomasstrba September 27, 2024 13:02
@brindy brindy requested a review from Bunn September 27, 2024 13:25
Copy link
Contributor

@Bunn Bunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested using a fake set of history items (provided by Brindy)

            if HistoryCoordinator.shared.history?.isEmpty == true {
                for i in 0 ..< 1000 {
                    let fakeDomain = "prefix\(Int.random(in: 0..<100)).com"
                    let url = URL(string: "https://\(fakeDomain)/\(UUID().uuidString)")!
                    for _ in 0 ..< Int.random(in: 1..<3) {
                        HistoryCoordinator.shared.addVisit(of: url)
                    }
                    HistoryCoordinator.shared.updateTitleIfNeeded(title: "Title of \(fakeDomain)", url: url)
                    HistoryCoordinator.shared.commitChanges(url: url)

                    if 0 == (i % 100) {
                        print("*** added \(i) history entries so far")
                    }
                }
            }

LGTM

@brindy brindy merged commit b60b38b into main Sep 27, 2024
14 checks passed
@brindy brindy deleted the brindy/fix-suggestions-performance branch September 27, 2024 14:38
brindy added a commit to duckduckgo/iOS that referenced this pull request Sep 27, 2024
<!--
Note: This checklist is a reminder of our shared engineering
expectations. Feel free to change it, although assigning a GitHub
reviewer and the items in bold are required.

⚠️ If you're an external contributor, please file an issue first before
working on a PR, as we can't guarantee that we will accept your changes
if they haven't been discussed ahead of time. Thanks!
-->

Task/Issue URL:
https://app.asana.com/0/1201048563534612/1208413716679959/f
Tech Design URL:
CC:

**Description**:
Bump BSK to include suggestions performance fix.

**Steps to test this PR**:
1. see duckduckgo/BrowserServicesKit#1008

* [ ] Does this PR satisfy our [Definition of
Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)?

**Copy Testing**:

* [ ] Use of correct apostrophes in new copy, ie `’` rather than `'`

**Orientation Testing**:

* [ ] Portrait
* [ ] Landscape

**Device Testing**:

* [ ] iPhone SE (1st Gen)
* [ ] iPhone 8
* [ ] iPhone X
* [ ] iPhone 14 Pro
* [ ] iPad

**OS Testing**:

* [ ] iOS 15
* [ ] iOS 16
* [ ] iOS 17

**Theme Testing**:

* [ ] Light theme
* [ ] Dark theme

---
###### Internal references:
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)
@Bioharzard
Copy link

Required:

Task/Issue URL: https://app.asana.com/0/1201048563534612/1208413716679959/f
iOS PR: duckduckgo/iOS#3405
macOS PR: duckduckgo/macos-browser#3353
What kind of version bump will this require?: Major/Minor/Patch

Optional:

Tech Design URL:
CC:

Description:
Fix suggestions performance issue by just looking at the first 100 sorted local results.

Steps to test this PR:

  1. Run the app on a local large data source (history entries > 1000 entries ideally)
  2. Use autocomplete and check that it doesn't time out for a broad range of matches

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.

3 participants