Davidkutu/finder speedup (microsoft#11311)

* search for the kernel by name in the path before opening the json file * Race the slow search options and wait for both in case an undefined wins. Use interpreterLocator to search for cached interpreters. * updated unit tests * reuse duplicated code

Author: David Kutugata

src/client/datascience/kernel-launcher/kernelFinder.ts

@@ -2,14 +2,19 @@
 // Licensed under the MIT License.
 'use strict';
 
-import { inject, injectable } from 'inversify';
+import { inject, injectable, named } from 'inversify';
 import * as path from 'path';
 import { InterpreterUri } from '../../common/installer/types';
 import { traceError, traceInfo } from '../../common/logger';
 import { IFileSystem, IPlatformService } from '../../common/platform/types';
 import { IExtensionContext, IPathUtils, Resource } from '../../common/types';
 import { isResource } from '../../common/utils/misc';
-import { IInterpreterService, PythonInterpreter } from '../../interpreter/contracts';
+import {
+ IInterpreterLocatorService,
+ IInterpreterService,
+ KNOWN_PATH_SERVICE,
+ PythonInterpreter
+} from '../../interpreter/contracts';
 import { IJupyterKernelSpec } from '../types';
 import { IKernelFinder } from './types';
 
@@ -38,6 +43,9 @@ export class KernelFinder implements IKernelFinder {
 
  constructor(
  @inject(IInterpreterService) private interpreterService: IInterpreterService,
+ @inject(IInterpreterLocatorService)
+ @named(KNOWN_PATH_SERVICE)
+ private readonly interpreterLocator: IInterpreterLocatorService,
  @inject(IPlatformService) private platformService: IPlatformService,
  @inject(IFileSystem) private file: IFileSystem,
  @inject(IPathUtils) private readonly pathUtils: IPathUtils,
@@ -62,20 +70,24 @@ export class KernelFinder implements IKernelFinder {
  }
 
  if (kernelSpec) {
+ // tslint:disable-next-line: no-floating-promises
+ this.writeCache(this.cache);
  return kernelSpec;
  }
 
- const kernelSearches = [
- this.interpreterService.getInterpreters(resource).then((interpreters) => {
- const interpreterPaths = interpreters.map((interp) => interp.path);
- return this.findInterpreterPath(interpreterPaths, kernelName);
- }),
- this.findDiskPath(kernelName)
- ];
+ const diskSearch = this.findDiskPath(kernelName);
+ const interpreterSearch = this.interpreterLocator.getInterpreters(resource, false).then((interpreters) => {
+ const interpreterPaths = interpreters.map((interp) => interp.path);
+ return this.findInterpreterPath(interpreterPaths, kernelName);
+ });
+
+ let result = await Promise.race([diskSearch, interpreterSearch]);
+ if (!result) {
+ const both = await Promise.all([diskSearch, interpreterSearch]);
+ result = both[0] ? both[0] : both[1];
+ }
 
- const result = await Promise.all(kernelSearches);
- const spec = result.find((sp) => sp?.name === kernelName);
- foundKernel = spec ? spec : await this.getDefaultKernelSpec(resource);
+ foundKernel = result ? result : await this.getDefaultKernelSpec(resource);
  } else {
  foundKernel = await this.getDefaultKernelSpec(resource);
  }
@@ -87,7 +99,7 @@ export class KernelFinder implements IKernelFinder {
 
  private async getKernelSpecFromActiveInterpreter(
  resource: Resource,
- kernelName?: string
+ kernelName: string
  ): Promise<IJupyterKernelSpec | undefined> {
  this.activeInterpreter = await this.interpreterService.getActiveInterpreter(resource);
 
@@ -99,47 +111,15 @@ export class KernelFinder implements IKernelFinder {
  }
  }
 
- private async getKernelSpec(kernelPath: string, kernelName?: string): Promise<IJupyterKernelSpec | undefined> {
+ private async getKernelSpec(kernelPath: string, kernelName: string): Promise<IJupyterKernelSpec | undefined> {
  try {
- const kernels = await this.file.getSubDirectories(kernelPath);
+ const paths = await this.file.getSubDirectories(kernelPath);
 
- if (kernels.length === 0) {
+ if (paths.length === 0) {
  return undefined;
  }
 
- // If no kernel name is included, return the first kernel
- if (!kernelName) {
- try {
- const kernelSpec: IJupyterKernelSpec = JSON.parse(
- await this.file.readFile(path.join(kernels[0], 'kernel.json'))
- );
- this.cache.push(kernelSpec);
- return kernelSpec;
- } catch (e) {
- traceError('Invalid kernel.json', e);
- return undefined;
- }
- }
-
- let spec: IJupyterKernelSpec | undefined;
- const promises = kernels.map(async (kernel) => {
- try {
- const kernelSpec: IJupyterKernelSpec = JSON.parse(
- await this.file.readFile(path.join(kernel, 'kernel.json'))
- );
- this.cache.push(kernelSpec);
- if (kernelSpec.name === kernelName) {
- spec = kernelSpec;
- }
- return kernelSpec;
- } catch (e) {
- traceError('Invalid kernel.json', e);
- return undefined;
- }
- });
-
- await Promise.all(promises);
- return spec;
+ return this.getKernelSpecFromDisk(paths, kernelName);
  } catch {
  traceInfo(`The path ${kernelPath} does not exist.`);
  return undefined;

src/test/datascience/kernelFinder.unit.test.ts

@@ -12,10 +12,16 @@ import { Architecture } from '../../client/common/utils/platform';
 import { KernelFinder } from '../../client/datascience/kernel-launcher/kernelFinder';
 import { IKernelFinder } from '../../client/datascience/kernel-launcher/types';
 import { IJupyterKernelSpec } from '../../client/datascience/types';
-import { IInterpreterService, InterpreterType, PythonInterpreter } from '../../client/interpreter/contracts';
+import {
+ IInterpreterLocatorService,
+ IInterpreterService,
+ InterpreterType,
+ PythonInterpreter
+} from '../../client/interpreter/contracts';
 
 suite('Kernel Finder', () => {
  let interpreterService: typemoq.IMock<IInterpreterService>;
+ let interpreterLocator: typemoq.IMock<IInterpreterLocatorService>;
  let fileSystem: typemoq.IMock<IFileSystem>;
  let platformService: typemoq.IMock<IPlatformService>;
  let pathUtils: typemoq.IMock<IPathUtils>;
@@ -39,17 +45,22 @@ suite('Kernel Finder', () => {
  .setup((fs) => fs.writeFile(typemoq.It.isAnyString(), typemoq.It.isAnyString()))
  .returns(() => Promise.resolve());
  fileSystem.setup((fs) => fs.getSubDirectories(typemoq.It.isAnyString())).returns(() => Promise.resolve(['']));
+ fileSystem
+ .setup((fs) => fs.search(typemoq.It.isAnyString(), typemoq.It.isAnyString()))
+ .returns((param: string) => Promise.resolve([param]));
  }
 
  setup(() => {
  interpreterService = typemoq.Mock.ofType<IInterpreterService>();
- interpreterService
- .setup((is) => is.getInterpreters(typemoq.It.isAny()))
- .returns(() => Promise.resolve(interpreters));
  interpreterService
  .setup((is) => is.getActiveInterpreter(typemoq.It.isAny()))
  .returns(() => Promise.resolve(activeInterpreter));
 
+ interpreterLocator = typemoq.Mock.ofType<IInterpreterLocatorService>();
+ interpreterLocator
+ .setup((il) => il.getInterpreters(typemoq.It.isAny(), typemoq.It.isAny()))
+ .returns(() => Promise.resolve(interpreters));
+
  fileSystem = typemoq.Mock.ofType<IFileSystem>();
  platformService = typemoq.Mock.ofType<IPlatformService>();
  platformService.setup((ps) => ps.isWindows).returns(() => true);
@@ -84,6 +95,7 @@ suite('Kernel Finder', () => {
 
  kernelFinder = new KernelFinder(
  interpreterService.object,
+ interpreterLocator.object,
  platformService.object,
  fileSystem.object,
  pathUtils.object,