-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: expose codebase indexing concurrency settings in UI #7510
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
Conversation
- Add configurable concurrency settings to codebase-index types - Update config-manager to read and provide concurrency settings - Modify DirectoryScanner to accept and use configurable concurrency values - Add VS Code configuration properties for the new settings - Add localization strings for user-friendly descriptions - Add tests for the new configuration functionality This allows users to optimize indexing performance based on their hardware capabilities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed my own code and found issues. Shocking, I know.
Review Findings
Critical Issues
-
Unused import in constants/index.ts - The import of
CODEBASE_INDEX_DEFAULTSappears to be unused. The constants (PARSING_CONCURRENCY, MAX_PENDING_BATCHES, BATCH_PROCESSING_CONCURRENCY) are still hardcoded instead of referencing the defaults from the types package. -
Missing runtime validation - The DirectoryScanner accepts config values but doesn't validate they're within acceptable bounds. Consider adding defensive validation.
Suggestions
-
Documentation opportunity - Consider adding JSDoc comments to the DirectoryScannerConfig interface to explain what each setting controls and their performance implications.
-
Test uncertainty - The test in config-manager-concurrency.spec.ts line 135 has a comment suggesting uncertainty about whether concurrency changes should require restart. This should be clarified.
Minor Notes
- The implementation correctly follows existing patterns and includes proper test coverage.
- The localization strings are properly added for the new settings.
Overall, the PR successfully addresses the issue requirements but needs some cleanup around constant usage and validation.
|
We recently exposed the batch threshold, we have experimented with the settings and found out that they rarely increase the processing speed before hitting rate limits or timeouts with the embeddings providers. I don't see any value in exposing these settings unless we have prove that changing one does decrease indexing time. |
|
@daniel-lxs I think that exposing those settings would allow the user to increase embedding speeds when running locally, cause currently I see no difference between using 0.7B and 7B model from qwen for embedding |
Description
This PR addresses Issue #7509 by exposing the codebase indexing concurrency settings in the VS Code extension settings UI, allowing users to optimize indexing performance based on their hardware capabilities.
Changes
Configuration Schema
codebase-index.tstype definitions:codebaseIndexParsingConcurrency: Number of files to parse in parallel (1-50, default: 10)codebaseIndexMaxPendingBatches: Maximum batches to accumulate before waiting (1-100, default: 20)codebaseIndexBatchProcessingConcurrency: Number of batches to process in parallel (1-50, default: 10)Implementation
CodeIndexConfigManagerto read and provide the new concurrency settingsDirectoryScannerto accept and use configurable concurrency values instead of hardcoded constantspackage.jsonwith appropriate defaults and constraintsTesting
Impact
Testing Instructions
Related Issue
Fixes #7509
Review Notes
The implementation follows existing patterns in the codebase and includes proper validation, error handling, and test coverage. The review tool analysis showed 95% confidence with a PROCEED recommendation.
Important
Expose codebase indexing concurrency settings in VS Code extension UI for customizable performance optimization.
codebaseIndexParsingConcurrency,codebaseIndexMaxPendingBatches, andcodebaseIndexBatchProcessingConcurrencytocodebase-index.ts.package.jsonto include new settings with defaults and constraints.CodeIndexConfigManagerto handle new settings.DirectoryScannerto use configurable concurrency values.service-factory.tsto pass concurrency settings toDirectoryScanner.config-manager-concurrency.spec.tsfor default, custom, and boundary values.config-manager.spec.tsto include new settings in configuration tests.This description was created by
for 8cc9424. You can customize this summary. It will automatically update as commits are pushed.