Skip to content

Commit

Permalink
Speed up query results copying (#25866)
Browse files Browse the repository at this point in the history
* Conditionally copy query results in backend

* Remote dep on remote hosts

* Remove changes

* Remove changes that caused slowdown

* Prefer copying from UI process

* Revert "Prefer copying from UI process"

This reverts commit 86d9849.

* Set prefer provider copy setting to false for linux

* Revert "Set prefer provider copy setting to false for linux"

This reverts commit 550d24e.

* Default copy handling to UI for Linux

* Bump SqlOps-DataProtocolClient package to 2.0.1

* Bump STS Version to 5.0.20240912.2
  • Loading branch information
lewis-sanchez committed Sep 13, 2024
1 parent 2401c59 commit 03b386a
Show file tree
Hide file tree
Showing 23 changed files with 42 additions and 58 deletions.
2 changes: 1 addition & 1 deletion extensions/azuremonitor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@
"update-grammar": "node ../../build/npm/update-grammar.js Microsoft/vscode-azuremonitor ./syntaxes/azuremonitor.tmLanguage"
},
"dependencies": {
"dataprotocol-client": "github:Microsoft/sqlops-dataprotocolclient#2.0.0",
"dataprotocol-client": "github:Microsoft/sqlops-dataprotocolclient#2.0.1",
"figures": "^2.0.0",
"find-remove": "1.2.1",
"@microsoft/ads-service-downloader": "^1.2.1",
Expand Down
6 changes: 3 additions & 3 deletions extensions/azuremonitor/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ [email protected]:
resolved "https://registry.yarnpkg.com/concat-map/-/concat-map-0.0.1.tgz#d8a96bd77fd68df7793a73036a3ba0d5405d477b"
integrity sha1-2Klr13/Wjfd5OnMDajug1UBdR3s=

"dataprotocol-client@github:Microsoft/sqlops-dataprotocolclient#2.0.0":
version "2.0.0"
resolved "https://codeload.github.com/Microsoft/sqlops-dataprotocolclient/tar.gz/39d7544771a4f44e707d0a38eb2fda1f08793d08"
"dataprotocol-client@github:Microsoft/sqlops-dataprotocolclient#2.0.1":
version "2.0.1"
resolved "https://codeload.github.com/Microsoft/sqlops-dataprotocolclient/tar.gz/7e8dff4aee7e674516367421a3e84fe9a1aa37b1"
dependencies:
vscode-languageclient "5.2.1"

Expand Down
2 changes: 1 addition & 1 deletion extensions/datavirtualization/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@
"dependencies": {
"@microsoft/ads-extension-telemetry": "^3.0.1",
"@microsoft/ads-service-downloader": "^1.2.1",
"dataprotocol-client": "github:Microsoft/sqlops-dataprotocolclient#2.0.0",
"dataprotocol-client": "github:Microsoft/sqlops-dataprotocolclient#2.0.1",
"vscode-nls": "^5.2.0"
},
"devDependencies": {
Expand Down
6 changes: 3 additions & 3 deletions extensions/datavirtualization/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -504,9 +504,9 @@ [email protected]:
resolved "https://registry.yarnpkg.com/crypt/-/crypt-0.0.2.tgz#88d7ff7ec0dfb86f713dc87bbb42d044d3e6c41b"
integrity sha512-mCxBlsHFYh9C+HVpiEacem8FEBnMXgU9gy4zmNC+SXAZNB/1idgp/aulFJ4FgCi7GPEVbfyng092GqL2k2rmow==

"dataprotocol-client@github:Microsoft/sqlops-dataprotocolclient#2.0.0":
version "2.0.0"
resolved "https://codeload.github.com/Microsoft/sqlops-dataprotocolclient/tar.gz/39d7544771a4f44e707d0a38eb2fda1f08793d08"
"dataprotocol-client@github:Microsoft/sqlops-dataprotocolclient#2.0.1":
version "2.0.1"
resolved "https://codeload.github.com/Microsoft/sqlops-dataprotocolclient/tar.gz/7e8dff4aee7e674516367421a3e84fe9a1aa37b1"
dependencies:
vscode-languageclient "5.2.1"

Expand Down
2 changes: 1 addition & 1 deletion extensions/import/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
}
},
"dependencies": {
"dataprotocol-client": "github:Microsoft/sqlops-dataprotocolclient#2.0.0",
"dataprotocol-client": "github:Microsoft/sqlops-dataprotocolclient#2.0.1",
"htmlparser2": "^3.10.1",
"@microsoft/ads-service-downloader": "^1.2.1",
"@microsoft/ads-extension-telemetry": "^3.0.1",
Expand Down
2 changes: 1 addition & 1 deletion extensions/import/src/test/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export class TestQueryProvider implements azdata.QueryProvider {
saveResults(requestParams: azdata.SaveResultsRequestParams): Thenable<azdata.SaveResultRequestResult> {
throw new Error('Method not implemented.');
}
copyResults(requestParams: azdata.CopyResultsRequestParams): Thenable<azdata.CopyResultsRequestResult> {
copyResults(requestParams: azdata.CopyResultsRequestParams): Thenable<void> {
throw new Error('Method not implemented.');
}
setQueryExecutionOptions(ownerUri: string, options: azdata.QueryExecutionOptions): Thenable<void> {
Expand Down
6 changes: 3 additions & 3 deletions extensions/import/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -559,9 +559,9 @@ [email protected]:
resolved "https://registry.yarnpkg.com/crypt/-/crypt-0.0.2.tgz#88d7ff7ec0dfb86f713dc87bbb42d044d3e6c41b"
integrity sha512-mCxBlsHFYh9C+HVpiEacem8FEBnMXgU9gy4zmNC+SXAZNB/1idgp/aulFJ4FgCi7GPEVbfyng092GqL2k2rmow==

"dataprotocol-client@github:Microsoft/sqlops-dataprotocolclient#2.0.0":
version "2.0.0"
resolved "https://codeload.github.com/Microsoft/sqlops-dataprotocolclient/tar.gz/39d7544771a4f44e707d0a38eb2fda1f08793d08"
"dataprotocol-client@github:Microsoft/sqlops-dataprotocolclient#2.0.1":
version "2.0.1"
resolved "https://codeload.github.com/Microsoft/sqlops-dataprotocolclient/tar.gz/7e8dff4aee7e674516367421a3e84fe9a1aa37b1"
dependencies:
vscode-languageclient "5.2.1"

Expand Down
2 changes: 1 addition & 1 deletion extensions/kusto/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@
}
},
"dependencies": {
"dataprotocol-client": "github:Microsoft/sqlops-dataprotocolclient#2.0.0",
"dataprotocol-client": "github:Microsoft/sqlops-dataprotocolclient#2.0.1",
"figures": "^2.0.0",
"find-remove": "1.2.1",
"@microsoft/ads-service-downloader": "^1.2.1",
Expand Down
6 changes: 3 additions & 3 deletions extensions/kusto/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,9 @@ [email protected]:
resolved "https://registry.yarnpkg.com/concat-map/-/concat-map-0.0.1.tgz#d8a96bd77fd68df7793a73036a3ba0d5405d477b"
integrity sha1-2Klr13/Wjfd5OnMDajug1UBdR3s=

"dataprotocol-client@github:Microsoft/sqlops-dataprotocolclient#2.0.0":
version "2.0.0"
resolved "https://codeload.github.com/Microsoft/sqlops-dataprotocolclient/tar.gz/39d7544771a4f44e707d0a38eb2fda1f08793d08"
"dataprotocol-client@github:Microsoft/sqlops-dataprotocolclient#2.0.1":
version "2.0.1"
resolved "https://codeload.github.com/Microsoft/sqlops-dataprotocolclient/tar.gz/7e8dff4aee7e674516367421a3e84fe9a1aa37b1"
dependencies:
vscode-languageclient "5.2.1"

Expand Down
2 changes: 1 addition & 1 deletion extensions/mssql/config.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"downloadUrl": "https://github.com/Microsoft/sqltoolsservice/releases/download/{#version#}/microsoft.sqltools.servicelayer-{#fileName#}",
"version": "5.0.20240724.1",
"version": "5.0.20240912.2",
"downloadFileNames": {
"Windows_86": "win-x86-net8.0.zip",
"Windows_64": "win-x64-net8.0.zip",
Expand Down
2 changes: 1 addition & 1 deletion extensions/mssql/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1712,7 +1712,7 @@
"@aws-sdk/client-s3": "^3.490.0",
"@microsoft/ads-extension-telemetry": "^3.0.1",
"@microsoft/ads-service-downloader": "^1.2.1",
"dataprotocol-client": "github:Microsoft/sqlops-dataprotocolclient#2.0.0",
"dataprotocol-client": "github:Microsoft/sqlops-dataprotocolclient#2.0.1",
"find-remove": "1.2.1",
"vscode-languageclient": "5.2.1",
"vscode-nls": "^4.0.0"
Expand Down
6 changes: 3 additions & 3 deletions extensions/mssql/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1559,9 +1559,9 @@ [email protected]:
resolved "https://registry.yarnpkg.com/crypt/-/crypt-0.0.2.tgz#88d7ff7ec0dfb86f713dc87bbb42d044d3e6c41b"
integrity sha512-mCxBlsHFYh9C+HVpiEacem8FEBnMXgU9gy4zmNC+SXAZNB/1idgp/aulFJ4FgCi7GPEVbfyng092GqL2k2rmow==

"dataprotocol-client@github:Microsoft/sqlops-dataprotocolclient#2.0.0":
version "2.0.0"
resolved "https://codeload.github.com/Microsoft/sqlops-dataprotocolclient/tar.gz/39d7544771a4f44e707d0a38eb2fda1f08793d08"
"dataprotocol-client@github:Microsoft/sqlops-dataprotocolclient#2.0.1":
version "2.0.1"
resolved "https://codeload.github.com/Microsoft/sqlops-dataprotocolclient/tar.gz/7e8dff4aee7e674516367421a3e84fe9a1aa37b1"
dependencies:
vscode-languageclient "5.2.1"

Expand Down
2 changes: 1 addition & 1 deletion extensions/sql-migration/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@
]
},
"dependencies": {
"dataprotocol-client": "github:Microsoft/sqlops-dataprotocolclient#2.0.0",
"dataprotocol-client": "github:Microsoft/sqlops-dataprotocolclient#2.0.1",
"@microsoft/ads-service-downloader": "^1.2.1",
"@microsoft/ads-extension-telemetry": "^3.0.1",
"uuid": "^8.3.2",
Expand Down
6 changes: 3 additions & 3 deletions extensions/sql-migration/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,9 @@ chownr@^2.0.0:
resolved "https://registry.yarnpkg.com/chownr/-/chownr-2.0.0.tgz#15bfbe53d2eab4cf70f18a8cd68ebe5b3cb1dece"
integrity sha512-bIomtDF5KGpdogkLd9VspvFzk9KfpyyGlS8YFVZl7TGPBHL5snIOnxeshwVgPteQ9b4Eydl+pVbIyE1DcvCWgQ==

"dataprotocol-client@github:Microsoft/sqlops-dataprotocolclient#2.0.0":
version "2.0.0"
resolved "https://codeload.github.com/Microsoft/sqlops-dataprotocolclient/tar.gz/39d7544771a4f44e707d0a38eb2fda1f08793d08"
"dataprotocol-client@github:Microsoft/sqlops-dataprotocolclient#2.0.1":
version "2.0.1"
resolved "https://codeload.github.com/Microsoft/sqlops-dataprotocolclient/tar.gz/7e8dff4aee7e674516367421a3e84fe9a1aa37b1"
dependencies:
vscode-languageclient "5.2.1"

Expand Down
9 changes: 1 addition & 8 deletions src/sql/azdata.proposed.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -894,13 +894,6 @@ declare module 'azdata' {
selections: SelectionRange[];
}

export interface CopyResultsRequestResult {
/**
* Result string from copy operation
*/
results: string;
}

export interface QueryProvider {
/**
* Notify clients that the URI for a connection has been changed.
Expand All @@ -912,7 +905,7 @@ declare module 'azdata' {
* ADS will use this if 'supportCopyResultsToClipboard' property is set to true in the provider contribution point in extension's package.json.
* Otherwise, The default handler will load all the selected data to ADS and perform the copy operation.
*/
copyResults?(requestParams: CopyResultsRequestParams): Thenable<CopyResultsRequestResult>;
copyResults?(requestParams: CopyResultsRequestParams): Thenable<void>;
}

export enum DataProviderType {
Expand Down
2 changes: 1 addition & 1 deletion src/sql/workbench/api/browser/mainThreadDataProtocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ export class MainThreadDataProtocol extends Disposable implements MainThreadData
return Promise.resolve(self._serializationService.saveAs(requestParams.resultFormat, requestParams.filePath, undefined, true));
}
},
copyResults(requestParams: azdata.CopyResultsRequestParams): Promise<azdata.CopyResultsRequestResult> {
copyResults(requestParams: azdata.CopyResultsRequestParams): Promise<void> {
return Promise.resolve(self._proxy.$copyResults(handle, requestParams));
},
initializeEdit(ownerUri: string, schemaName: string, objectName: string, objectType: string, rowLimit: number, queryString: string): Promise<void> {
Expand Down
2 changes: 1 addition & 1 deletion src/sql/workbench/api/common/extHostDataProtocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ export class ExtHostDataProtocol extends ExtHostDataProtocolShape {
return this._resolveProvider<azdata.QueryProvider>(handle).saveResults(requestParams);
}

override $copyResults(handle: number, requestParams: azdata.CopyResultsRequestParams): Thenable<azdata.CopyResultsRequestResult> {
override $copyResults(handle: number, requestParams: azdata.CopyResultsRequestParams): Thenable<void> {
const provider = this._resolveProvider<azdata.QueryProvider>(handle);
if (provider.copyResults) {
return provider.copyResults(requestParams);
Expand Down
2 changes: 1 addition & 1 deletion src/sql/workbench/api/common/sqlExtHost.protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ export abstract class ExtHostDataProtocolShape {
/**
* Copies the selected data to clipboard.
*/
$copyResults(handle: number, requestParams: azdata.CopyResultsRequestParams): Thenable<azdata.CopyResultsRequestResult> { throw ni(); }
$copyResults(handle: number, requestParams: azdata.CopyResultsRequestParams): Thenable<void> { throw ni(); }

/**
* Commits all pending edits in an edit session
Expand Down
5 changes: 3 additions & 2 deletions src/sql/workbench/contrib/query/browser/query.contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
import { IConfigurationRegistry, Extensions as ConfigExtensions, IConfigurationNode } from 'vs/platform/configuration/common/configurationRegistry';
import { MenuId, MenuRegistry, registerAction2 } from 'vs/platform/actions/common/actions';
import { KeyMod, KeyCode, KeyChord } from 'vs/base/common/keyCodes';
import * as platform from 'vs/base/common/platform';
import { KeybindingsRegistry, KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry';
import { ContextKeyExpr, ContextKeyEqualsExpr } from 'vs/platform/contextkey/common/contextkey';

Expand Down Expand Up @@ -335,8 +336,8 @@ const queryEditorConfiguration: IConfigurationNode = {
},
'queryEditor.results.preferProvidersCopyHandler': {
'type': 'boolean',
'description': localize('queryEditor.results.preferProvidersCopyHandler', "Whether the copy result request should be handled by the query provider when it is supported. The default value is true, set this to false to force all copy handling to be done by Azure Data Studio."),
'default': true
'description': localize('queryEditor.results.preferProvidersCopyHandler', "Whether the copy result request should be handled by the query provider when it is supported. The default value is true for Windows and Mac, and false for Linux, set this to false to force all copy handling to be done by Azure Data Studio."),
'default': platform.isLinux ? false : true
},
'queryEditor.results.inMemoryDataProcessingThreshold': {
'type': 'number',
Expand Down
6 changes: 3 additions & 3 deletions src/sql/workbench/services/query/common/queryManagement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export interface IQueryManagementService {
changeConnectionUri(newUri: string, oldUri: string): Promise<void>;
saveResults(requestParams: azdata.SaveResultsRequestParams): Promise<azdata.SaveResultRequestResult>;
setQueryExecutionOptions(uri: string, options: azdata.QueryExecutionOptions): Promise<void>;
copyResults(params: azdata.CopyResultsRequestParams): Promise<azdata.CopyResultsRequestResult>;
copyResults(params: azdata.CopyResultsRequestParams): Promise<void>;

// Callbacks
onQueryComplete(result: azdata.QueryExecuteCompleteNotificationResult): void;
Expand Down Expand Up @@ -108,7 +108,7 @@ export interface IQueryRequestHandler {
disposeQuery(ownerUri: string): Promise<void>;
connectionUriChanged(newUri: string, oldUri: string): Promise<void>;
saveResults(requestParams: azdata.SaveResultsRequestParams): Promise<azdata.SaveResultRequestResult>;
copyResults(requestParams: azdata.CopyResultsRequestParams): Promise<azdata.CopyResultsRequestResult>;
copyResults(requestParams: azdata.CopyResultsRequestParams): Promise<void>;
setQueryExecutionOptions(ownerUri: string, options: azdata.QueryExecutionOptions): Promise<void>;

// Edit Data actions
Expand Down Expand Up @@ -364,7 +364,7 @@ export class QueryManagementService implements IQueryManagementService {
});
}

public copyResults(requestParams: azdata.CopyResultsRequestParams): Promise<azdata.CopyResultsRequestResult> {
public copyResults(requestParams: azdata.CopyResultsRequestParams): Promise<void> {
return this._runAction(requestParams.ownerUri, (runner) => {
return runner.copyResults(requestParams);
});
Expand Down
8 changes: 1 addition & 7 deletions src/sql/workbench/services/query/common/queryModelService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import Severity from 'vs/base/common/severity';
import EditQueryRunner from 'sql/workbench/services/editData/common/editQueryRunner';
import { IRange } from 'vs/editor/common/core/range';
import { ServerConnID } from 'sql/workbench/services/query/common/query';
import { ClipboardData, IClipboardService } from 'vs/platform/clipboard/common/clipboardService';
import { IQueryManagementService } from 'sql/workbench/services/query/common/queryManagement';

const selectionSnippetMaxLen = 100;
Expand Down Expand Up @@ -88,7 +87,6 @@ export class QueryModelService implements IQueryModelService {
@IQueryManagementService protected queryManagementService: IQueryManagementService,
@IInstantiationService private _instantiationService: IInstantiationService,
@INotificationService private _notificationService: INotificationService,
@IClipboardService private _clipboardService: IClipboardService,
@ILogService private _logService: ILogService
) {
this._queryInfoMap = new Map<string, QueryInfo>();
Expand Down Expand Up @@ -178,11 +176,7 @@ export class QueryModelService implements IQueryModelService {

public async copyResults(uri: string, selection: Slick.Range[], batchId: number, resultId: number, includeHeaders?: boolean): Promise<void> {
if (this._queryInfoMap.has(uri)) {
const results = await this._queryInfoMap.get(uri)!.queryRunner!.copyResults(selection, batchId, resultId, includeHeaders);
let clipboardData: ClipboardData = {
text: results.results
};
this._clipboardService.write(clipboardData);
return this._queryInfoMap.get(uri)!.queryRunner!.copyResults(selection, batchId, resultId, includeHeaders);
}
}

Expand Down
12 changes: 4 additions & 8 deletions src/sql/workbench/services/query/common/queryRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { ResultSerializer, SaveFormat } from 'sql/workbench/services/query/commo

import * as azdata from 'azdata';
import * as nls from 'vs/nls';
import { ClipboardData, IClipboardService } from 'vs/platform/clipboard/common/clipboardService';
import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService';
import * as types from 'vs/base/common/types';
import { Disposable } from 'vs/base/common/lifecycle';
import { INotificationService } from 'vs/platform/notification/common/notification';
Expand Down Expand Up @@ -494,8 +494,8 @@ export default class QueryRunner extends Disposable {
* @param resultId The result id of the result to copy from
* @param includeHeaders [Optional]: Should column headers be included in the copy selection
*/
async copyResults(selections: Slick.Range[], batchId: number, resultId: number, includeHeaders?: boolean): Promise<azdata.CopyResultsRequestResult> {
return this.queryManagementService.copyResults({
async copyResults(selections: Slick.Range[], batchId: number, resultId: number, includeHeaders?: boolean): Promise<void> {
await this.queryManagementService.copyResults({
ownerUri: this.uri,
batchIndex: batchId,
resultSetIndex: resultId,
Expand Down Expand Up @@ -617,11 +617,7 @@ export class QueryGridDataProvider implements IGridDataProvider {

private async handleCopyRequestByProvider(selections: Slick.Range[], includeHeaders?: boolean): Promise<void> {
executeCopyWithNotification(this._notificationService, this._configurationService, selections, async () => {
let results = await this.queryRunner.copyResults(selections, this.batchId, this.resultSetId, this.shouldIncludeHeaders(includeHeaders));
let clipboardData: ClipboardData = {
text: results.results
}
this._clipboardService.write(clipboardData);
await this.queryRunner.copyResults(selections, this.batchId, this.resultSetId, this.shouldIncludeHeaders(includeHeaders));
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export class TestQueryManagementService implements IQueryManagementService {
saveResults(requestParams: azdata.SaveResultsRequestParams): Promise<azdata.SaveResultRequestResult> {
throw new Error('Method not implemented.');
}
copyResults(params: azdata.CopyResultsRequestParams): Promise<azdata.CopyResultsRequestResult> {
copyResults(params: azdata.CopyResultsRequestParams): Promise<void> {
throw new Error('Method not implemented.');
}
setQueryExecutionOptions(uri: string, options: azdata.QueryExecutionOptions): Promise<void> {
Expand Down

0 comments on commit 03b386a

Please sign in to comment.