Skip to content

Commit d6e916e

Browse files
committed
fix: improve repl/variable/completion performance for TS
Fixes #1433 There were a couple problems: - TS uses a `customDescriptionGenerator`. Previously this was being invoked on each individual property. I've merged this with the logic we added for custom `toString()` representations, so that we run it only once for an object and return a map of its properties. - TS has thousands of variables in each scope. We call `Runtime.getProperties` to get scope variables to use in completions. It seems like getProperties, which does not have any 'limit' parameter, is blocking. We still need this to do sensible completions, but now each stacktrace caches its completions instead of getting new ones on every request. With these changes, there may still be a little REPL stall on the first evaluation (until the first completions call finishes, which takes 3-5s on my macbook), but after that it should be appropriately snappy.
1 parent d812305 commit d6e916e

File tree

7 files changed

+125
-93
lines changed

7 files changed

+125
-93
lines changed

src/adapter/completions.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ export class Completions {
324324
...params,
325325
returnByValue: true,
326326
throwOnSideEffect: false,
327-
expression: enumeratePropertiesTemplate(
327+
expression: enumeratePropertiesTemplate.expr(
328328
`(${expression})`,
329329
JSON.stringify(prefix),
330330
JSON.stringify(isInGlobalScope),

src/adapter/stackTrace.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ export class StackFrame implements IFrameElement {
468468
return (scope.variables[scopeNumber] = variable);
469469
}
470470

471-
async completions(): Promise<Dap.CompletionItem[]> {
471+
public readonly completions = once(async (): Promise<Dap.CompletionItem[]> => {
472472
if (!this._scope) return [];
473473
const variableStore = this._thread.pausedVariables();
474474
if (!variableStore) {
@@ -479,14 +479,14 @@ export class StackFrame implements IFrameElement {
479479
for (let scopeNumber = 0; scopeNumber < this._scope.chain.length; scopeNumber++) {
480480
promises.push(
481481
this._scopeVariable(scopeNumber, this._scope).then(async scopeVariable => {
482-
const variables = await variableStore.getVariables({
482+
const variables = await variableStore.getVariableNames({
483483
variablesReference: scopeVariable.id,
484484
});
485-
return variables.map(variable => ({ label: variable.name, type: 'property' }));
485+
return variables.map(label => ({ label: label, type: 'property' }));
486486
}),
487487
);
488488
}
489489
const completions = await Promise.all(promises);
490490
return ([] as Dap.CompletionItem[]).concat(...completions);
491-
}
491+
});
492492
}

src/adapter/templates/getStringyProps.ts

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,31 +2,66 @@
22
* Copyright (C) Microsoft Corporation. All rights reserved.
33
*--------------------------------------------------------*/
44

5-
import { remoteFunction } from '.';
5+
import { templateFunction } from '.';
66

77
/**
88
* Gets a mapping of property names with a custom `.toString()` method
99
* to their string representations.
1010
*/
11-
export const getStringyProps = remoteFunction(function (this: unknown, maxLength: number) {
11+
export const getStringyProps = templateFunction(function (
12+
this: unknown,
13+
maxLength: number,
14+
customToString: (defaultRepr: string) => unknown,
15+
) {
1216
const out: Record<string, string> = {};
17+
const defaultPlaceholder = '<<default preview>>';
1318
if (typeof this !== 'object' || !this) {
1419
return out;
1520
}
1621

1722
for (const [key, value] of Object.entries(this)) {
23+
if (customToString) {
24+
try {
25+
const repr = customToString.call(value, defaultPlaceholder);
26+
if (repr !== defaultPlaceholder) {
27+
out[key] = String(repr);
28+
continue;
29+
}
30+
} catch (e) {
31+
out[key] = `${e} (couldn't describe ${key})`;
32+
continue;
33+
}
34+
}
35+
1836
if (typeof value === 'object' && value && !String(value.toString).includes('[native code]')) {
1937
const str = String(value);
2038
if (!str.startsWith('[object ')) {
2139
out[key] = str.length >= maxLength ? str.slice(0, maxLength) + '…' : str;
40+
continue;
2241
}
2342
}
2443
}
2544

2645
return out;
2746
});
2847

29-
export const getToStringIfCustom = remoteFunction(function (this: unknown, maxLength: number) {
48+
export const getToStringIfCustom = templateFunction(function (
49+
this: unknown,
50+
maxLength: number,
51+
customToString: (defaultRepr: string) => unknown,
52+
) {
53+
if (customToString) {
54+
try {
55+
const defaultPlaceholder = '<<default preview>>';
56+
const repr = customToString.call(this, defaultPlaceholder);
57+
if (repr !== defaultPlaceholder) {
58+
return String(repr);
59+
}
60+
} catch (e) {
61+
return `${e} (couldn't describe object)`;
62+
}
63+
}
64+
3065
if (typeof this === 'object' && this && !String(this.toString).includes('[native code]')) {
3166
const str = String(this);
3267
if (!str.startsWith('[object ')) {

src/adapter/templates/index.ts

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ import { SourceConstants } from '../sources';
1414
export const getSourceSuffix = () =>
1515
`\n//# sourceURL=eval-${randomBytes(4).toString('hex')}${SourceConstants.InternalExtension}\n`;
1616

17+
export type TemplateFunction<A extends unknown[]> = {
18+
expr: (...args: A) => string;
19+
decl: (...args: A) => string;
20+
};
21+
1722
/**
1823
* Creates a template for the given function that replaces its arguments
1924
* and generates a string to be executed where it takes expressions to be
@@ -43,21 +48,21 @@ export const getSourceSuffix = () =>
4348
* })();
4449
* ```
4550
*/
46-
export function templateFunction<A>(fn: (a: A) => void): (a: string) => string;
47-
export function templateFunction<A, B>(fn: (a: A, b: B) => void): (a: string, b: string) => string;
51+
export function templateFunction<A>(fn: (a: A) => void): TemplateFunction<[string]>;
52+
export function templateFunction<A, B>(
53+
fn: (a: A, b: B) => void,
54+
): TemplateFunction<[string, string]>;
4855
export function templateFunction<A, B, C>(
4956
fn: (a: A, b: B, c: C) => void,
50-
): (a: string, b: string, c: string) => string;
51-
export function templateFunction<Args extends unknown[]>(fn: string): (...args: Args) => string;
57+
): TemplateFunction<[string, string, string]>;
58+
export function templateFunction<Args extends unknown[]>(fn: string): TemplateFunction<Args>;
5259
export function templateFunction<Args extends unknown[]>(
5360
fn: string | ((...args: Args) => void),
54-
): (...args: string[]) => string {
61+
): TemplateFunction<string[]> {
5562
return templateFunctionStr('' + fn);
5663
}
5764

58-
export function templateFunctionStr<Args extends string[]>(
59-
stringified: string,
60-
): (...args: Args) => string {
65+
function templateFunctionStr<Args extends string[]>(stringified: string): TemplateFunction<Args> {
6166
const decl = parseExpressionAt(stringified, 0, {
6267
ecmaVersion: 'latest',
6368
locations: true,
@@ -76,10 +81,14 @@ export function templateFunctionStr<Args extends string[]>(
7681
});
7782

7883
const { start, end } = decl.body as unknown as Node;
79-
return (...args) => `(() => {
84+
const inner = (args: string[]) => `
8085
${args.map((a, i) => `let ${params[i]} = ${a}`).join('; ')};
8186
${stringified.slice(start + 1, end - 1)}
82-
})();${getSourceSuffix()}`;
87+
`;
88+
return {
89+
expr: (...args: Args) => `(()=>{${inner(args)}})();\n${getSourceSuffix()}`,
90+
decl: (...args: Args) => `function(){${inner(args)};\n${getSourceSuffix()}}`,
91+
};
8392
}
8493

8594
/**

src/adapter/templates/serializeForClipboard.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ export const serializeForClipboardTmpl = templateFunction(function (
141141

142142
export const serializeForClipboard = remoteFunction<[number], string>(`
143143
function(spaces2) {
144-
const result = ${serializeForClipboardTmpl('this', 'spaces2')};
144+
const result = ${serializeForClipboardTmpl.decl('this', 'spaces2')};
145145
return result;
146146
}
147147
`);

src/adapter/threads.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ export class Thread implements IVariableStoreLocationProvider {
471471
const params: Cdp.Runtime.EvaluateParams =
472472
args.context === 'clipboard'
473473
? {
474-
expression: serializeForClipboardTmpl(args.expression, '2'),
474+
expression: serializeForClipboardTmpl.expr(args.expression, '2'),
475475
includeCommandLineAPI: true,
476476
returnByValue: true,
477477
objectGroup: 'console',

0 commit comments

Comments
 (0)