improvement: improve performance of bit-import on lane (teambit#5931)

davidfirst · web-flow · commit a5a87eb4aaa5 · 2022-06-08T14:33:08.000+03:00
- import only "delta" when on a lane with the same way it is done for main. meaning, send to the server the current hash and only fetch if the server has a newer hash, in which case, fetch the delta between the two. (disabled temporarily, until the server is deployed with these changes).

- in case the components exist locally but their version are unbuilt, avoid fetching them again from the remote with all their dependencies, unless their `builtStatus` has changed and is now successful.
diff --git a/e2e/harmony/sign.e2e.ts b/e2e/harmony/sign.e2e.ts
@@ -117,7 +117,7 @@ describe('sign command', function () {
     });
     describe('running bit import on the workspace', () => {
       before(() => {
-        helper.command.importAllComponents();
+        helper.command.import('--all-history');
       });
       it('should bring the updated Version from the remote', () => {
         const comp1 = helper.command.catComponent(`${helper.scopes.remote}/bar@latest`);
diff --git a/src/api/scope/lib/fetch.ts b/src/api/scope/lib/fetch.ts
@@ -1,7 +1,7 @@
 import { Readable } from 'stream';
 import { LaneId } from '@teambit/lane-id';
 import { BitIds } from '../../../bit-id';
-import { POST_SEND_OBJECTS, PRE_SEND_OBJECTS } from '../../../constants';
+import { LATEST_BIT_VERSION, POST_SEND_OBJECTS, PRE_SEND_OBJECTS } from '../../../constants';
 import HooksManager from '../../../hooks';
 import logger from '../../../logger/logger';
 import { loadScope, Scope } from '../../../scope';
@@ -13,13 +13,16 @@ import {
   ObjectsReadableGenerator,
 } from '../../../scope/objects/objects-readable-generator';
 import { LaneNotFound } from './exceptions/lane-not-found';
+import { Lane } from '../../../scope/models';
 
 export type FETCH_TYPE = 'component' | 'lane' | 'object' | 'component-delta';
 export type FETCH_OPTIONS = {
   type: FETCH_TYPE;
   withoutDependencies: boolean; // default - true
   includeArtifacts: boolean; // default - false
   allowExternal: boolean; // allow fetching components of other scope from this scope. needed for lanes.
+  laneId?: string; // relevant for "component-delta" type to know where to find the component latest
+  onlyIfBuilt?: boolean; // relevant when fetching with deps. if true, and the component wasn't built successfully, don't fetch it.
 };
 
 const HooksManagerInstance = HooksManager.getInstance();
@@ -50,7 +53,7 @@ export default async function fetch(
   switch (fetchOptions.type) {
     case 'component': {
       const bitIds: BitIds = BitIds.deserialize(ids);
-      const { withoutDependencies, includeArtifacts, allowExternal } = fetchOptions;
+      const { withoutDependencies, includeArtifacts, allowExternal, onlyIfBuilt } = fetchOptions;
       const collectParents = !withoutDependencies;
       const scopeComponentsImporter = ScopeComponentsImporter.getInstance(scope);
       const getComponentsWithOptions = async (): Promise<ComponentWithCollectOptions[]> => {
@@ -63,7 +66,7 @@ export default async function fetch(
             collectParents,
           }));
         }
-        const versionsDependencies = await scopeComponentsImporter.fetchWithDeps(bitIds, allowExternal);
+        const versionsDependencies = await scopeComponentsImporter.fetchWithDeps(bitIds, allowExternal, onlyIfBuilt);
         return versionsDependencies
           .map((versionDep) => [
             {
@@ -89,7 +92,9 @@ export default async function fetch(
     case 'component-delta': {
       const bitIdsWithHashToStop: BitIds = BitIds.deserialize(ids);
       const scopeComponentsImporter = ScopeComponentsImporter.getInstance(scope);
-      const bitIdsLatest = bitIdsWithHashToStop.toVersionLatest();
+      const laneId = fetchOptions.laneId ? LaneId.parse(fetchOptions.laneId) : null;
+      const lane = laneId ? await scope.loadLane(laneId) : null;
+      const bitIdsLatest = bitIdsToLatest(bitIdsWithHashToStop, lane);
       const importedComponents = await scopeComponentsImporter.fetchWithoutDeps(
         bitIdsLatest,
         fetchOptions.allowExternal
@@ -151,3 +156,16 @@ export default async function fetch(
   logger.debug('scope.fetch returns readable');
   return objectsReadableGenerator.readable;
 }
+
+function bitIdsToLatest(bitIds: BitIds, lane: Lane | null) {
+  if (!lane) {
+    return bitIds.toVersionLatest();
+  }
+  const laneIds = lane.toBitIds();
+  return BitIds.fromArray(
+    bitIds.map((bitId) => {
+      const inLane = laneIds.searchWithoutVersion(bitId);
+      return inLane || bitId.changeVersion(LATEST_BIT_VERSION);
+    })
+  );
+}
diff --git a/src/consumer/component-ops/import-components.ts b/src/consumer/component-ops/import-components.ts
@@ -79,13 +79,14 @@ export default class ImportComponents {
   options: ImportOptions;
   // @ts-ignore AUTO-ADDED-AFTER-MIGRATION-PLEASE-FIX!
   mergeStatus: { [id: string]: FilesStatus };
-  private laneObjects: Lane[] = [];
+  private laneObjects: Lane[];
   private divergeData: Array<ModelComponent> = [];
   // @ts-ignore
   constructor(consumer: Consumer, options: ImportOptions) {
     this.consumer = consumer;
     this.scope = consumer.scope;
     this.options = options;
+    this.laneObjects = this.options.lanes ? (this.options.lanes.lanes as Lane[]) : [];
   }
 
   importComponents(): Promise<ImportResult> {
@@ -96,10 +97,49 @@ export default class ImportComponents {
       this.options.installNpmPackages = false;
       this.options.saveDependenciesAsComponents = true;
     }
-    if (!this.options.lanes && (!this.options.ids || R.isEmpty(this.options.ids))) {
-      return this.importAccordingToBitMap();
+    if (this.options.lanes && !this.options.ids.length) {
+      // @todo: uncomment this once the code is deployed to the server
+      // return this.importObjectsOnLane();
+      return this.importSpecificComponents();
     }
-    return this.importSpecificComponents();
+    if (this.options.ids.length) {
+      return this.importSpecificComponents();
+    }
+    return this.importAccordingToBitMap();
+  }
+
+  async importObjectsOnLane(): Promise<ImportResult> {
+    if (!this.laneObjects.length || !this.options.objectsOnly || this.consumer.isLegacy) {
+      throw new Error(`importObjectsOnLane should work on Harmony and have laneObjects and objectsOnly=true`);
+    }
+    if (this.laneObjects.length > 1) {
+      throw new Error(`importObjectsOnLane does not support more than one lane`);
+    }
+    const lane = this.laneObjects[0];
+    const bitIds: BitIds = await this.getBitIds();
+    logger.debug(`importObjectsOnLane, Lane: ${lane.id()}, Ids: ${bitIds.toString()}`);
+    const beforeImportVersions = await this._getCurrentVersions(bitIds);
+    const componentsWithDependencies = await this.consumer.importComponentsObjectsHarmony(
+      bitIds,
+      false,
+      this.options.allHistory,
+      lane
+    );
+
+    // merge the lane objects
+    const mergeAllLanesResults = await pMapSeries(this.laneObjects, (laneObject) =>
+      this.scope.sources.mergeLane(laneObject, true)
+    );
+    const mergedLanes = mergeAllLanesResults.map((result) => result.mergeLane);
+    await Promise.all(mergedLanes.map((mergedLane) => this.scope.lanes.saveLane(mergedLane)));
+
+    const componentsWithDependenciesFiltered = this._filterComponentsWithLowerVersions(componentsWithDependencies);
+    await this._fetchDivergeData(componentsWithDependenciesFiltered);
+    this._throwForDivergedHistory();
+    await this._writeToFileSystem(componentsWithDependenciesFiltered);
+    await this._saveLaneDataIfNeeded(componentsWithDependenciesFiltered);
+    const importDetails = await this._getImportDetails(beforeImportVersions, componentsWithDependencies);
+    return { dependencies: componentsWithDependenciesFiltered, importDetails };
   }
 
   async importSpecificComponents(): Promise<ImportResult> {
@@ -187,7 +227,6 @@ export default class ImportComponents {
     if (!this.options.lanes) {
       throw new Error(`getBitIdsForLanes: this.options.lanes must be set`);
     }
-    this.laneObjects = this.options.lanes.lanes as Lane[];
     const bitIdsFromLane = BitIds.fromArray(this.laneObjects.flatMap((lane) => lane.toBitIds()));
 
     if (!this.options.ids.length) {
diff --git a/src/consumer/consumer.ts b/src/consumer/consumer.ts
@@ -434,14 +434,15 @@ export default class Consumer {
   async importComponentsObjectsHarmony(
     ids: BitIds,
     fromOriginalScope = false,
-    allHistory = false
+    allHistory = false,
+    lane?: Lane
   ): Promise<ComponentWithDependencies[]> {
     const scopeComponentsImporter = ScopeComponentsImporter.getInstance(this.scope);
-    await scopeComponentsImporter.importManyDeltaWithoutDeps(ids, allHistory);
+    await scopeComponentsImporter.importManyDeltaWithoutDeps(ids, allHistory, lane);
     loader.start(`import ${ids.length} components with their dependencies (if missing)`);
     const versionDependenciesArr: VersionDependencies[] = fromOriginalScope
       ? await scopeComponentsImporter.importManyFromOriginalScopes(ids)
-      : await scopeComponentsImporter.importMany({ ids });
+      : await scopeComponentsImporter.importMany({ ids, lanes: lane ? [lane] : undefined });
     const componentWithDependencies = await multipleVersionDependenciesToConsumer(
       versionDependenciesArr,
       this.scope.objects
diff --git a/src/scope/component-ops/scope-components-importer.ts b/src/scope/component-ops/scope-components-importer.ts
@@ -28,6 +28,7 @@ import VersionDependencies from '../version-dependencies';
 import { BitObjectList } from '../objects/bit-object-list';
 import { ObjectFetcher } from '../objects-fetcher/objects-fetcher';
 import { concurrentComponentsLimit } from '../../utils/concurrency';
+import { BuildStatus } from '../../constants';
 
 const removeNils = R.reject(R.isNil);
 
@@ -82,9 +83,10 @@ export default class ScopeComponentsImporter {
   }): Promise<VersionDependencies[]> {
     logger.debugAndAddBreadCrumb(
       'importMany',
-      `cache ${cache}, throwForDependencyNotFound: ${throwForDependencyNotFound}. ids: {ids}`,
+      `cache ${cache}, throwForDependencyNotFound: ${throwForDependencyNotFound}. ids: {ids}, lanes: {lanes}`,
       {
         ids: ids.toString(),
+        lanes: lanes ? lanes.map((lane) => lane.id()).join(', ') : undefined,
       }
     );
     const idsToImport = compact(ids.filter((id) => id.hasScope()));
@@ -232,7 +234,7 @@ export default class ScopeComponentsImporter {
    * delta between the local head and the remote head. mainly to improve performance
    * not applicable and won't work for legacy. for legacy, refer to importManyWithAllVersions
    */
-  async importManyDeltaWithoutDeps(ids: BitIds, allHistory = false): Promise<void> {
+  async importManyDeltaWithoutDeps(ids: BitIds, allHistory = false, lane?: Lane): Promise<void> {
     logger.debugAndAddBreadCrumb('importManyDeltaWithoutDeps', `Ids: {ids}`, { ids: ids.toString() });
     const idsWithoutNils = BitIds.uniqFromArray(compact(ids));
     if (R.isEmpty(idsWithoutNils)) return;
@@ -243,8 +245,7 @@ export default class ScopeComponentsImporter {
         // remove the version to fetch it with all versions.
         return id.changeVersion(undefined);
       }
-      // @todo: fix to consider local lane
-      const remoteLaneId = LaneId.from(DEFAULT_LANE, id.scope as string);
+      const remoteLaneId = lane ? lane.toLaneId() : LaneId.from(DEFAULT_LANE, id.scope as string);
       const remoteHead = await this.repo.remoteLanes.getRef(remoteLaneId, id);
       if (!remoteHead) {
         return id.changeVersion(undefined);
@@ -258,7 +259,7 @@ export default class ScopeComponentsImporter {
       }
       return id.changeVersion(remoteHead.toString());
     });
-    const groupedIds = groupByScopeName(idsToFetch);
+    const groupedIds = lane ? groupByLanes(idsToFetch, [lane]) : groupByScopeName(idsToFetch);
     const idsOnlyDelta = idsToFetch.filter((id) => id.hasVersion());
     const idsAllHistory = idsToFetch.filter((id) => !id.hasVersion());
     const remotesCount = Object.keys(groupedIds).length;
@@ -273,8 +274,10 @@ export default class ScopeComponentsImporter {
       {
         type: 'component-delta',
         withoutDependencies: true,
+        laneId: lane ? lane.id() : undefined,
       },
-      idsToFetch
+      idsToFetch,
+      lane ? [lane] : undefined
     ).fetchFromRemoteAndWrite();
   }
 
@@ -338,15 +341,15 @@ export default class ScopeComponentsImporter {
     return compact(componentVersionArr);
   }
 
-  async fetchWithDeps(ids: BitIds, allowExternal: boolean): Promise<VersionDependencies[]> {
+  async fetchWithDeps(ids: BitIds, allowExternal: boolean, onlyIfBuild = false): Promise<VersionDependencies[]> {
     logger.debugAndAddBreadCrumb('fetchWithDeps', `ids: {ids}`, { ids: ids.toString() });
     if (!allowExternal) this.throwIfExternalFound(ids);
     // avoid race condition of getting multiple "fetch" requests, which later translates into
     // multiple getExternalMany calls, which saves objects and write refs files at the same time
     return this.fetchWithDepsMutex.runExclusive(async () => {
       logger.debug('fetchWithDeps, acquiring a lock');
       const localDefs: ComponentDef[] = await this.sources.getMany(ids);
-      const versionDeps = await this.multipleCompsDefsToVersionDeps(localDefs);
+      const versionDeps = await this.multipleCompsDefsToVersionDeps(localDefs, undefined, onlyIfBuild);
       logger.debug('fetchWithDeps, releasing the lock');
       return versionDeps;
     });
@@ -453,7 +456,8 @@ export default class ScopeComponentsImporter {
 
   private async multipleCompsDefsToVersionDeps(
     compsDefs: ComponentDef[],
-    lanes: Lane[] = []
+    lanes: Lane[] = [],
+    onlyIfBuilt = false
   ): Promise<VersionDependencies[]> {
     const concurrency = concurrentComponentsLimit();
     const componentsWithVersionsWithNulls = await pMap(
@@ -474,6 +478,14 @@ export default class ScopeComponentsImporter {
         if (!version) {
           throw new Error(`ScopeComponentImporter, expect ${id.toString()} to have a Version object`);
         }
+        if (onlyIfBuilt && version.buildStatus !== BuildStatus.Succeed) {
+          logger.debug(
+            `multipleCompsDefsToVersionDeps, id: ${id.toString()} is skipped because its build-status is ${
+              version.buildStatus
+            }`
+          );
+          return null;
+        }
 
         return { componentVersion: versionComp, versionObj: version };
       },
@@ -508,21 +520,29 @@ export default class ScopeComponentsImporter {
     lanes: Lane[] = []
   ): Promise<VersionDependencies[]> {
     if (!ids.length) return [];
-    logger.debugAndAddBreadCrumb('ScopeComponentsImporter.getExternalMany', `fetching from remote scope. Ids: {ids}`, {
-      ids: ids.join(', '),
-    });
+    logger.debugAndAddBreadCrumb(
+      'ScopeComponentsImporter.getExternalMany',
+      `fetching from remote scope. Ids: {ids}, Lanes: {lanes}`,
+      {
+        ids: ids.join(', '),
+        lanes: lanes.map((lane) => lane.id()).join(', '),
+      }
+    );
     const context = {};
     ids.forEach((id) => {
       if (id.isLocal(this.scope.name))
         throw new Error(`getExternalMany expects to get external ids only, got ${id.toString()}`);
     });
     enrichContextFromGlobal(Object.assign({}, { requestedBitIds: ids.map((id) => id.toString()) }));
+    // avoid re-fetching the components with all deps if they're still un-built
+    const onlyIfBuilt = ids.every((id) => this.sources.isUnBuiltInCache(id));
     await new ObjectFetcher(
       this.repo,
       this.scope,
       remotes,
       {
         withoutDependencies: false,
+        onlyIfBuilt,
       },
       ids,
       lanes,
diff --git a/src/scope/models/lane.ts b/src/scope/models/lane.ts
@@ -2,7 +2,7 @@ import { v4 } from 'uuid';
 import { isHash } from '@teambit/component-version';
 import { LaneId, DEFAULT_LANE, LANE_REMOTE_DELIMITER } from '@teambit/lane-id';
 import { Scope } from '..';
-import { BitId } from '../../bit-id';
+import { BitId, BitIds } from '../../bit-id';
 import { CFG_USER_EMAIL_KEY, CFG_USER_NAME_KEY, PREVIOUS_DEFAULT_LANE } from '../../constants';
 import ValidationError from '../../error/validation-error';
 import logger from '../../logger/logger';
@@ -181,8 +181,8 @@ export default class Lane extends BitObject {
     );
     return { merged, unmerged };
   }
-  toBitIds(): BitId[] {
-    return this.components.map((c) => c.id.changeVersion(c.head.toString()));
+  toBitIds(): BitIds {
+    return BitIds.fromArray(this.components.map((c) => c.id.changeVersion(c.head.toString())));
   }
   toLaneId() {
     return new LaneId({ scope: this.scope, name: this.name });
diff --git a/src/scope/repositories/sources.ts b/src/scope/repositories/sources.ts
@@ -49,7 +49,7 @@ export type MergeResult = {
   mergedVersions: string[];
 };
 
-const MAX_AGE_UN_BUILT_COMPS_CACHE = 60 * 1000;
+const MAX_AGE_UN_BUILT_COMPS_CACHE = 60 * 1000; // 1 min
 
 export default class SourceRepository {
   scope: Scope;
@@ -155,6 +155,10 @@ export default class SourceRepository {
     return returnComponent(version as Version);
   }
 
+  isUnBuiltInCache(bitId: BitId): boolean {
+    return Boolean(this.cacheUnBuiltIds.get(bitId.toString()));
+  }
+
   async _findComponent(component: ModelComponent): Promise<ModelComponent | undefined> {
     try {
       const foundComponent = await this.objects().load(component.hash());
