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

Improve Search Performance #4239

Closed
JustinGrote opened this issue Jan 21, 2021 · 8 comments · Fixed by #5424
Closed

Improve Search Performance #4239

JustinGrote opened this issue Jan 21, 2021 · 8 comments · Fixed by #5424

Comments

@JustinGrote
Copy link

https://github.com/shilangyu/scoop-search has changed my experience with scoop, the slow search has always been one of my pain points as a user.

I'd be willing to PR improvement to the search engine to incorporate the lessons learnd here if there is interest.

@Bios-Marcel
Copy link

It appears that the problem is that we parse the whole json. Maybe we can try to switch this to an approach where we only parse until we find the bin section?

@rashil2000
Copy link
Member

I'd be willing to PR improvement to the search engine to incorporate the lessons learnd here if there is interest.

Yes! Please make a PR when you can.

@bartvanandel
Copy link

I was annoyed by this myself, so I've just put some effort in writing a fix for this. Will publish a PR soon.

@burgr033
Copy link

burgr033 commented Jun 27, 2023

I managed to reduce search time by almost 99%
Because I wanted to keep the core search intact (and I don't understand a bit what is going on under the hood), I built myself an additional command, that builds a cache file containing names, binaries, buckets and versions of all apps in all local buckets, and added a parameter to scoop search (--offline) that just searches this index file.

╭─sb@tealc ~
╰─$ time scoop search lazygit
Results from local buckets...

Name    Version Source Binaries
----    ------- ------ --------
lazygit 0.38.2  extras
00:00:18.2134557

╭─sb@tealc ~
╰─$ time scoop search lazygit --offline
Results from cached list...

Name    Version Source Binaries
----    ------- ------ --------
lazygit 0.38.2  extras lazygit.exe
00:00:00.1915792

For my use case this really is enough. I do not know if this is a valid approach for any of you, i just wanted to share the idea.

Cheers

@Bios-Marcel
Copy link

Awesome idea @cigh033

Sounds like something worth adding, but not invoke by default.

@Yakiyo
Copy link

Yakiyo commented Aug 31, 2023

@cigh033 generating a cache file would indeed make subsequent searches faster, but the initial invokation would need the same, if not more amount of time as the original search command right? and wouldn't the cache file need to be updated after certain intervals too?

@rashil2000
Copy link
Member

I narrowed down the regression in search times to the cmdlet manifest_path in manifest.ps1. With the current version

function manifest_path($app, $bucket) {
    (Get-ChildItem (Find-BucketDirectory $bucket) -Filter "$(sanitary_path $app).json" -Recurse).FullName
}

I get the 20 second search times but with the previous

function manifest_path($app, $bucket) {
    fullpath "$(Find-BucketDirectory $bucket)\$(sanitary_path $app).json"
}

It goes down to 8 seconds.

It seems like the new version is searching for a file in all subdirectories. Is this really necessary? Don't all manifests have to be always located in bucket/?

Originally posted by @mathijshenquet in #5615 (comment)

Don't all manifests have to be always located in bucket/?

Historically, manifests were also present in the root directory. However most buckets now follow the convention of having a bucket/ subdirectory, in order to keep the repo cleaner.

@xialvjun
Copy link

Just an advice: maybe we can use sqlite or moreover we can use a git like sql databse like https://github.com/dolthub/dolt

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 a pull request may close this issue.

7 participants