fix: skipFiles being slow in remotes (#1532)

Fixes https://github.com/microsoft/vscode/issues/170412

We were not rebasing remote paths correctly in the initial launch,
which caused the fallback logic to kick in and manually apply skipFiles
to every single script (and recompile globs), which was quite slow.

And add a test that I wrote while barking up the wrong tree.

src/adapter/scriptSkipper/implementation.ts

@@ -13,6 +13,7 @@ import { node15InternalsPrefix } from '../../common/node15Internal';
 import { memoizeLast, trailingEdgeThrottle, truthy } from '../../common/objUtils';
 import * as pathUtils from '../../common/pathUtils';
 import { getDeferred, IDeferred } from '../../common/promiseUtil';
+import { ISourcePathResolver } from '../../common/sourcePathResolver';
 import { escapeRegexSpecialChars } from '../../common/stringUtils';
 import * as urlUtils from '../../common/urlUtils';
 import { AnyLaunchConfiguration } from '../../configuration';
@@ -48,12 +49,15 @@ function preprocessNodeInternals(userSkipPatterns: ReadonlyArray<string>): strin
   return nodeInternalPatterns.length > 0 ? nodeInternalPatterns : undefined;
 }
 
-function preprocessAuthoredGlobs(userSkipPatterns: ReadonlyArray<string>): string[] {
+function preprocessAuthoredGlobs(
+  spr: ISourcePathResolver,
+  userSkipPatterns: ReadonlyArray<string>,
+): string[] {
   const authoredGlobs = userSkipPatterns
     .filter(pattern => !pattern.includes('<node_internals>'))
     .map(pattern =>
       urlUtils.isAbsolute(pattern)
-        ? urlUtils.absolutePathToFileUrl(pattern)
+        ? urlUtils.absolutePathToFileUrlWithDetection(spr.rebaseLocalToRemote(pattern))
         : pathUtils.forceForwardSlashes(pattern),
     )
     .map(urlUtils.lowerCaseInsensitivePath);
@@ -105,6 +109,7 @@ export class ScriptSkipper {
 
   constructor(
     @inject(AnyLaunchConfiguration) { skipFiles }: AnyLaunchConfiguration,
+    @inject(ISourcePathResolver) sourcePathResolver: ISourcePathResolver,
     @inject(ILogger) private readonly logger: ILogger,
     @inject(ICdpApi) private readonly cdp: Cdp.Api,
     @inject(ITarget) target: ITarget,
@@ -115,7 +120,7 @@ export class ScriptSkipper {
       this._normalizeUrl(key),
     );
 
-    this._authoredGlobs = preprocessAuthoredGlobs(skipFiles);
+    this._authoredGlobs = preprocessAuthoredGlobs(sourcePathResolver, skipFiles);
     this._nodeInternalsGlobs = preprocessNodeInternals(skipFiles);
 
     this._initNodeInternals(target); // Purposely don't wait, no need to slow things down
@@ -195,7 +200,7 @@ export class ScriptSkipper {
       return true;
     }
 
-    return this._testSkipAuthored(this._normalizeUrl(url));
+    return this._testSkipAuthored(url);
   }
 
   private async _updateSourceWithSkippedSourceMappedSources(

src/common/objUtils.test.ts

@@ -0,0 +1,33 @@
+/*---------------------------------------------------------
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ *--------------------------------------------------------*/
+
+import { expect } from 'chai';
+import { memoizeLast } from './objUtils';
+
+describe('objUtils', () => {
+  it('memoizeLast', () => {
+    let calls = 0;
+    const fn = memoizeLast((m: number[]) => {
+      calls++;
+      return m.reduce((a, b) => a + b, 0);
+    });
+
+    const a = [1, 2, 3];
+    const b = [4, 5, 6];
+    expect(fn(a)).to.equal(6);
+    expect(calls).to.equal(1);
+
+    expect(fn(a)).to.equal(6);
+    expect(calls).to.equal(1);
+
+    expect(fn(b)).to.equal(15);
+    expect(calls).to.equal(2);
+
+    expect(fn(b)).to.equal(15);
+    expect(calls).to.equal(2);
+
+    expect(fn(a)).to.equal(6);
+    expect(calls).to.equal(3);
+  });
+});

src/common/urlUtils.ts

@@ -299,7 +299,8 @@ export function fileUrlToNetworkPath(urlOrPath: string): string {
   return urlOrPath;
 }
 
-// TODO: this does not escape/unescape special characters, but it should.
+// TODO: this does not escape/unescape special characters
+/** @deprecated consider absolutePathToFileUrlWithDetection instead */
 export function absolutePathToFileUrl(absolutePath: string): string {
   if (platform === 'win32') {
     return 'file:///' + platformPathToUrlPath(absolutePath);
@@ -307,6 +308,18 @@ export function absolutePathToFileUrl(absolutePath: string): string {
   return 'file://' + platformPathToUrlPath(absolutePath);
 }
 
+/**
+ * Absolte path former that detects the platform based on the absolutePath
+ * itself, rather than the platform where the debugger is running. This is
+ * different from {@link absolutePathToFileUrl}, but should be more correct.
+ */
+export function absolutePathToFileUrlWithDetection(absolutePath: string): string {
+  if (!absolutePath.startsWith('/')) {
+    return 'file:///' + platformPathToUrlPath(absolutePath);
+  }
+  return 'file://' + platformPathToUrlPath(absolutePath);
+}
+
 /**
  * Returns whether the path is a Windows or posix path.
  */

src/test/benchmark/scriptSkipper.ts

@@ -8,12 +8,15 @@ import { Source } from '../../adapter/sources';
 import Connection from '../../cdp/connection';
 import { NullTransport } from '../../cdp/nullTransport';
 import { Logger } from '../../common/logging/logger';
+import { upcastPartial } from '../../common/objUtils';
+import { ISourcePathResolver } from '../../common/sourcePathResolver';
 import { AnyLaunchConfiguration } from '../../configuration';
 import { ITarget } from '../../targets/targets';
 import { NullTelemetryReporter } from '../../telemetry/nullTelemetryReporter';
 
 const skipper = new ScriptSkipper(
   { skipFiles: ['<node_internals>/**', '/foo/*.js'] } as unknown as AnyLaunchConfiguration,
+  upcastPartial<ISourcePathResolver>({ rebaseLocalToRemote: p => p }),
   Logger.null,
   new Connection(new NullTransport(), Logger.null, new NullTelemetryReporter()).createSession(''),
   {