feature: make number of ripgrep threads configurable #213511

SimonSiefke · 2024-05-26T16:58:51Z

fixes #206030.

Testing

For testing, I used the process explorer to see the ripgrep args being used, e.g. if the --threads argument is passed correctly to ripgrep.

With many threads

In settings.json, search.threads is commented out. By default, multiple threads are used. The ripgrep cli args don't include the --threads option:

With one thread

In settings.json, search.threads is set to 1. The ripgrep cli args include --threads 1:

gjsjohnmurray

LGTM but unit test failures need fixing.

andreamah

Thanks for the contribution! Just some comments.

src/vs/workbench/contrib/search/browser/search.contribution.ts

src/vs/workbench/services/search/common/search.ts

src/vs/workbench/services/search/common/searchExtTypes.ts

src/vs/workbench/services/search/node/ripgrepFileSearch.ts

src/vs/workbench/services/search/node/ripgrepTextSearchEngine.ts

src/vs/workbench/services/search/common/queryBuilder.ts

Co-authored-by: Andrea Mah <31675041+andreamah@users.noreply.github.com>

…ickAccess.ts

…chEditor.ts

…Manager.ts

andreamah

Should be the last round of comments, thanks so much for making changes so quickly!

src/vs/workbench/services/search/common/searchExtTypesInternal.ts

src/vs/workbench/services/search/node/fileSearch.ts

andreamah · 2024-06-04T21:48:17Z

Okay, I'm testing it now- I realize that it says Num Threads here, which isn't the best

Could you change the outward-facing setting to maxThreads? Max Threads might look better.

andreamah · 2024-06-04T21:57:11Z

src/vs/workbench/api/node/extHostSearch.ts

 		@ILogService _logService: ILogService,
 	) {
 		super(extHostRpc, _uriTransformer, _logService);

 		const outputChannel = new OutputChannel('RipgrepSearchUD', this._logService);
-		this._disposables.add(this.registerTextSearchProvider(Schemas.vscodeUserData, new RipgrepSearchProvider(outputChannel)));
+		this._numThreadsPromise = _configurationService.getConfigProvider().then(provider => provider.getConfiguration('search').get<number>('ripgrep.numThreads'));


I just tested this out, and it seems that it won't update correctly if I change the setting. This is probably because the configuration is initialized once here and isn't updated when changed. I think you should be able to listen to onDidChangeConfiguration on ExtHostConfigProvider to listen for this.

Good point! I changed it to read the configuration value when the search starts so that always the latest configuration value is used.

andreamah · 2024-06-05T15:30:18Z

Should

vscode/src/vs/workbench/services/search/node/rawSearchService.ts

Line 125 in cbffb5d

const engine = new EngineClass(config);

also pass through numThreads?

andreamah · 2024-06-05T15:35:41Z

I noticed that the string of added args ends up being
--threads '3'
Should the number end up being a string?

andreamah · 2024-06-05T15:37:27Z

I noticed that the string of added args ends up being --threads '3' Should the number end up being a string?

I see both work on rg, so nvm on that.

SimonSiefke · 2024-06-05T15:55:26Z

Should vscode/src/vs/workbench/services/search/node/rawSearchService.ts also pass through numThreads?

Yes, great find! I added the property there also!

src/vs/workbench/api/node/extHostSearch.ts

andreamah

Looks good to me! Thanks for sticking with me on all my requested changes 🚀

SimonSiefke added 10 commits May 26, 2024 17:30

feature: add threads property

63ba89f

add setting

43ddea0

use setting

82da18b

debug

c1f5e28

add option

029a542

fix types

36c54b5

use commonquery

7f56456

rename argument

5123567

remove comma

b41019e

add threads for file search

37977b0

VSCodeTriageBot assigned andreamah May 26, 2024

gjsjohnmurray approved these changes May 26, 2024

View reviewed changes

set threads to undefined

3db497f

andreamah requested changes May 30, 2024

View reviewed changes

SimonSiefke and others added 16 commits May 30, 2024 21:48

Merge branch 'main' into feature/ripgrep-threads

1f9028d

update description

5a3d152

remove unused code

aa93896

remove unused code

118a9a6

Update src/vs/workbench/services/search/node/ripgrepFileSearch.ts

7977989

Co-authored-by: Andrea Mah <31675041+andreamah@users.noreply.github.com>

Update src/vs/workbench/services/search/node/ripgrepTextSearchEngine.ts

e10e9ae

Co-authored-by: Andrea Mah <31675041+andreamah@users.noreply.github.com>

use ICommonQueryBuilderOptions

dbf93b1

set threads property from caller

a8df3e8

simplify code

782aaba

Discard changes to src/vs/workbench/contrib/search/browser/anythingQu…

e35ce48

…ickAccess.ts

Discard changes to src/vs/workbench/contrib/searchEditor/browser/sear…

d1b74c8

…chEditor.ts

remove unused code

c1f070b

Discard changes to src/vs/workbench/services/search/common/textSearch…

ddc3155

…Manager.ts

add back code

a3d2c37

fix types

953862f

update formatting

02f2a65

SimonSiefke added 2 commits June 4, 2024 19:08

Merge remote-tracking branch 'origin/main' into feature/ripgrep-threads

f8e8f3a

Discard changes to src/vs/monaco.d.ts

b6c3171

andreamah requested changes Jun 4, 2024

View reviewed changes

src/vs/workbench/services/search/common/searchExtTypesInternal.ts Outdated Show resolved Hide resolved

src/vs/workbench/services/search/node/fileSearch.ts Outdated Show resolved Hide resolved

andreamah reviewed Jun 4, 2024

View reviewed changes

SimonSiefke added 7 commits June 5, 2024 09:03

Merge remote-tracking branch 'origin/main' into feature/ripgrep-threads

4efeb73

rename interface

38bcbbb

rename variable

6bef66d

rename setting

9029984

rename setting

b91f2b8

update settings

32a68d6

tsc

cbffb5d

SimonSiefke added 2 commits June 5, 2024 17:52

fix: pass threads property also below

8ef7b31

Merge branch 'main' into feature/ripgrep-threads

1306f94

andreamah reviewed Jun 5, 2024

View reviewed changes

src/vs/workbench/api/node/extHostSearch.ts Show resolved Hide resolved

SimonSiefke added 4 commits June 6, 2024 08:00

Merge branch 'main' into feature/ripgrep-threads

8fd3e11

cache setting

e890453

use affectsConfiguration

21e5b71

fix test

9d9bab4

andreamah approved these changes Jun 6, 2024

View reviewed changes

VSCodeTriageBot added this to the June 2024 milestone Jun 6, 2024

lramos15 approved these changes Jun 6, 2024

View reviewed changes

andreamah merged commit 0036990 into microsoft:main Jun 6, 2024
6 checks passed

andreamah mentioned this pull request Jun 24, 2024

ripgrep file search uses too many threads #206030

Closed

GregKruz approved these changes Jul 15, 2024

View reviewed changes

vs-code-engineering bot locked and limited conversation to collaborators Jul 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: make number of ripgrep threads configurable #213511

feature: make number of ripgrep threads configurable #213511

SimonSiefke commented May 26, 2024

gjsjohnmurray left a comment •

edited

Loading

andreamah left a comment

andreamah left a comment

andreamah commented Jun 4, 2024

andreamah Jun 4, 2024

SimonSiefke Jun 5, 2024

andreamah commented Jun 5, 2024

andreamah commented Jun 5, 2024

andreamah commented Jun 5, 2024

SimonSiefke commented Jun 5, 2024

andreamah left a comment

feature: make number of ripgrep threads configurable #213511

feature: make number of ripgrep threads configurable #213511

Conversation

SimonSiefke commented May 26, 2024

Testing

With many threads

With one thread

gjsjohnmurray left a comment • edited Loading

Choose a reason for hiding this comment

andreamah left a comment

Choose a reason for hiding this comment

andreamah left a comment

Choose a reason for hiding this comment

andreamah commented Jun 4, 2024

andreamah Jun 4, 2024

Choose a reason for hiding this comment

SimonSiefke Jun 5, 2024

Choose a reason for hiding this comment

andreamah commented Jun 5, 2024

andreamah commented Jun 5, 2024

andreamah commented Jun 5, 2024

SimonSiefke commented Jun 5, 2024

andreamah left a comment

Choose a reason for hiding this comment

gjsjohnmurray left a comment •

edited

Loading