Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

internal(component-loading) - change comp loading flow to improve performance #8970

Merged
merged 2 commits into from
Jun 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions scopes/scope/scope/scope-aspects-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,14 @@ needed-for: ${neededFor || '<unknown>'}`);
return { manifests: [], potentialPluginsIds: [] };
}
const components = await this.getNonLoadedAspects(nonVisitedId, lane);
// Removing this part for now as it's not needed for now
// If you see a case where it's needed, please consult Gilad.
// Adding all the envs ids to the array to support case when one (or more) of the aspects has custom aspect env
const customEnvsIds = components
.map((component) => this.envs.getEnvId(component))
.filter((envId) => !this.aspectLoader.isCoreEnv(envId));
// In case there is custom env we need to load it right away, otherwise we will fail during the require aspects
await this.getManifestsAndLoadAspects(customEnvsIds, undefined, lane);
// const customEnvsIds = components
// .map((component) => this.envs.getEnvId(component))
// .filter((envId) => !this.aspectLoader.isCoreEnv(envId));
// // In case there is custom env we need to load it right away, otherwise we will fail during the require aspects
// await this.getManifestsAndLoadAspects(customEnvsIds, undefined, lane);
visited.push(...nonVisitedId);
const manifests = await this.requireAspects(components, throwOnError, opts);
const potentialPluginsIds = compact(
Expand Down
22 changes: 15 additions & 7 deletions scopes/scope/scope/scope.main.runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,12 @@ export type LoadOptions = {
* In case the component we are loading is env, whether to load it as env (in a scope aspects capsule)
*/
loadEnvs?: boolean;

/**
* Should we load the components' aspects (this useful when you only want sometime to load the component itself
* as aspect but not its aspects)
*/
loadCompAspects?: boolean;
};

export type ScopeConfig = {
Expand Down Expand Up @@ -794,7 +800,7 @@ export class ScopeMain implements ComponentFactory {
async loadMany(
ids: ComponentID[],
lane?: Lane,
opts: LoadOptions = { loadApps: true, loadEnvs: true }
opts: LoadOptions = { loadApps: true, loadEnvs: true, loadCompAspects: true }
): Promise<Component[]> {
const components = await mapSeries(ids, (id) => this.load(id, lane, opts));
return compact(components);
Expand Down Expand Up @@ -1074,29 +1080,31 @@ export class ScopeMain implements ComponentFactory {
async load(
id: ComponentID,
lane?: Lane,
opts: LoadOptions = { loadApps: true, loadEnvs: true }
opts: LoadOptions = { loadApps: true, loadEnvs: true, loadCompAspects: true }
): Promise<Component | undefined> {
const component = await this.get(id);
if (!component) return undefined;
return this.loadCompAspects(component, lane, opts);
const optsWithDefaults = { loadApps: true, loadEnvs: true, loadCompAspects: true, ...opts };
return this.loadCompAspects(component, lane, optsWithDefaults);
}

async loadCompAspects(
component: Component,
lane?: Lane,
opts: LoadOptions = { loadApps: true, loadEnvs: true }
opts: LoadOptions = { loadApps: true, loadEnvs: true, loadCompAspects: true }
): Promise<Component> {
const aspectIds = component.state.aspects.ids;
const optsWithDefaults = { loadApps: true, loadEnvs: true, loadCompAspects: true, ...opts };
const aspectIds = optsWithDefaults.loadCompAspects ? component.state.aspects.ids : [];
// load components from type aspects as aspects.
// important! previously, this was running for any aspect, not only apps. (the if statement was `this.aspectLoader.isAspectComponent(component)`)
// Ran suggests changing it and if it breaks something, we'll document is and fix it.
if (opts.loadApps) {
if (optsWithDefaults.loadApps) {
const appData = component.state.aspects.get('teambit.harmony/application');
if (appData?.data?.appName) {
aspectIds.push(component.id.toString());
}
}
if (opts.loadEnvs) {
if (optsWithDefaults.loadEnvs) {
const envsData = component.state.aspects.get(EnvsAspect.id);
if (envsData?.data?.services || envsData?.data?.self || envsData?.data?.type === 'env') {
aspectIds.push(component.id.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type LoadGroupMetadata = {
core?: boolean;
aspects?: boolean;
seeders?: boolean;
envs?: boolean;
};

type GetAndLoadSlotOpts = ComponentLoadOptions & LoadGroupMetadata;
Expand Down Expand Up @@ -92,7 +93,11 @@ export class WorkspaceComponentLoader {
* Cache extension list for components. used by get many for perf improvements.
* And to make sure we load extensions first.
*/
private componentsExtensionsCache: InMemoryCache<{ extensions: ExtensionDataList; errors: Error[] | undefined }>;
private componentsExtensionsCache: InMemoryCache<{
extensions: ExtensionDataList;
errors: Error[] | undefined;
envId: string | undefined;
}>;

private componentLoadedSelfAsAspects: InMemoryCache<boolean>; // cache loaded components
constructor(
Expand Down Expand Up @@ -171,13 +176,13 @@ export class WorkspaceComponentLoader {
}
const groupsRes = compact(
await mapSeries(groupsToHandle, async (group, index) => {
const { scopeIds, workspaceIds, aspects, core, seeders } = group;
const { scopeIds, workspaceIds, aspects, core, seeders, envs } = group;
const groupDesc = `getMany-${callId} group ${index + 1}/${groupsToHandle.length} - ${loadGroupToStr(group)}`;
this.logger.profile(groupDesc);
if (!workspaceIds.length && !scopeIds.length) {
throw new Error('getAndLoadSlotOrdered - group has no ids to load');
}
const res = await this.getAndLoadSlot(workspaceIds, scopeIds, { ...loadOpts, core, seeders, aspects });
const res = await this.getAndLoadSlot(workspaceIds, scopeIds, { ...loadOpts, core, seeders, aspects, envs });
this.logger.profile(groupDesc);
// We don't want to return components that were not asked originally (we do want to load them)
if (!group.seeders) return undefined;
Expand All @@ -197,7 +202,9 @@ export class WorkspaceComponentLoader {
}

private async buildLoadGroups(workspaceScopeIdsMap: WorkspaceScopeIdsMap): Promise<Array<LoadGroup>> {
const allIds = [...workspaceScopeIdsMap.workspaceIds.values(), ...workspaceScopeIdsMap.scopeIds.values()];
const wsIds = Array.from(workspaceScopeIdsMap.workspaceIds.values());
const scopeIds = Array.from(workspaceScopeIdsMap.scopeIds.values());
const allIds = [...wsIds, ...scopeIds];
const groupedByIsCoreEnvs = groupBy(allIds, (id) => {
return this.envs.isCoreEnv(id.toStringWithoutVersion());
});
Expand All @@ -219,9 +226,30 @@ export class WorkspaceComponentLoader {
const allExtCompIds = Array.from(allExtIds.values());
await this.populateScopeAndExtensionsCache(allExtCompIds || [], workspaceScopeIdsMap);

const allExtIdsStr = allExtCompIds.map((id) => id.toString());
// const allExtIdsStr = allExtCompIds.map((id) => id.toString());

const envsIdsOfWsComps = new Set<string>();
wsIds.forEach((id) => {
const idStr = id.toString();
const fromCache = this.componentsExtensionsCache.get(idStr);
if (!fromCache || !fromCache.extensions) {
return;
}
const envId = fromCache.envId;
if (envId) {
envsIdsOfWsComps.add(envId);
}
});

const groupedByIsEnvOfWsComps = groupBy(allExtCompIds, (id) => {
const idStr = id.toString();
const withoutVersion = idStr.split('@')[0];
return envsIdsOfWsComps.has(idStr) || envsIdsOfWsComps.has(withoutVersion);
});
const notEnvOfWsCompsStrs = (groupedByIsEnvOfWsComps.false || []).map((id) => id.toString());

const groupedByIsExtOfAnother = groupBy(nonCoreEnvs, (id) => {
return allExtIdsStr.includes(id.toString());
return notEnvOfWsCompsStrs.includes(id.toString());
});
const extIdsFromTheList = (groupedByIsExtOfAnother.true || []).map((id) => id.toString());
const extsNotFromTheList: ComponentID[] = [];
Expand All @@ -240,15 +268,17 @@ export class WorkspaceComponentLoader {
core: false,
aspects: true,
seeders: true,
envs: false,
};
});

const groupsToHandle = [
// Always load first core envs
{ ids: groupedByIsCoreEnvs.true || [], core: true, aspects: true, seeders: true },
{ ids: extsNotFromTheList || [], core: false, aspects: true, seeders: false },
{ ids: groupedByIsCoreEnvs.true || [], core: true, aspects: true, seeders: true, envs: true },
{ ids: groupedByIsEnvOfWsComps.true || [], core: false, aspects: true, seeders: false, envs: true },
{ ids: extsNotFromTheList || [], core: false, aspects: true, seeders: false, envs: false },
...layeredExtGroups,
{ ids: groupedByIsExtOfAnother.false || [], core: false, aspects: false, seeders: true },
{ ids: groupedByIsExtOfAnother.false || [], core: false, aspects: false, seeders: true, envs: false },
];
const groupsByWsScope = groupsToHandle.map((group) => {
if (!group.ids?.length) return undefined;
Expand All @@ -261,6 +291,7 @@ export class WorkspaceComponentLoader {
core: group.core,
aspects: group.aspects,
seeders: group.seeders,
envs: group.envs,
};
});
return compact(groupsByWsScope);
Expand All @@ -287,9 +318,10 @@ export class WorkspaceComponentLoader {
loadOpts
);

const components = workspaceComponents.concat(scopeComponents);

const allExtensions: ExtensionDataList[] = components.map((component) => {
// If we are here it means we are on workspace, in that case we don't want to load
// aspects of scope components as aspects only aspects of workspace components
// const components = workspaceComponents.concat(scopeComponents);
const allExtensions: ExtensionDataList[] = workspaceComponents.map((component) => {
return component.state._consumer.extensions;
});

Expand Down Expand Up @@ -392,10 +424,15 @@ export class WorkspaceComponentLoader {
}
if (!this.componentsExtensionsCache.has(idStr) && workspaceScopeIdsMap.workspaceIds.has(idStr)) {
componentFromScope = componentFromScope || this.scopeComponentsCache.get(idStr);
const { extensions, errors } = await this.workspace.componentExtensions(id, componentFromScope, undefined, {
loadExtensions: false,
});
this.componentsExtensionsCache.set(idStr, { extensions, errors });
const { extensions, errors, envId } = await this.workspace.componentExtensions(
id,
componentFromScope,
undefined,
{
loadExtensions: false,
}
);
this.componentsExtensionsCache.set(idStr, { extensions, errors, envId });
}
});
}
Expand Down Expand Up @@ -537,11 +574,14 @@ export class WorkspaceComponentLoader {
// as when loading the next batch of components (next group) we won't have the envs loaded

try {
const scopeComponents = await this.workspace.scope.getMany(scopeIds);

// We don't want to load envs as part of this step, they will be loaded later
const scopeComponents = await this.workspace.scope.loadMany(scopeIds, undefined, {
loadApps: false,
loadEnvs: false,
});
// const scopeComponents = await this.workspace.scope.loadMany(scopeIds, undefined, {
// loadApps: false,
// loadEnvs: true,
// loadCompAspects: false,
// });
return {
workspaceComponents: components,
scopeComponents,
Expand Down Expand Up @@ -855,7 +895,7 @@ function sortKeys(obj: Object) {

function printGroupsToHandle(groupsToHandle: Array<LoadGroup>, logger: Logger): void {
groupsToHandle.forEach((group) => {
const { scopeIds, workspaceIds, aspects, core, seeders } = group;
const { scopeIds, workspaceIds, aspects, core, seeders, envs } = group;
logger.console(
`workspace-component-loader ~ groupsToHandle ${JSON.stringify(
{
Expand All @@ -864,6 +904,7 @@ function printGroupsToHandle(groupsToHandle: Array<LoadGroup>, logger: Logger):
aspects,
core,
seeders,
envs,
},
null,
2
Expand All @@ -873,12 +914,13 @@ function printGroupsToHandle(groupsToHandle: Array<LoadGroup>, logger: Logger):
}

function loadGroupToStr(loadGroup: LoadGroup): string {
const { scopeIds, workspaceIds, aspects, core, seeders } = loadGroup;
const { scopeIds, workspaceIds, aspects, core, seeders, envs } = loadGroup;

const attr: string[] = [];
if (aspects) attr.push('aspects');
if (core) attr.push('core');
if (seeders) attr.push('seeders');
if (envs) attr.push('envs');

return `workspaceIds: ${workspaceIds.length}, scopeIds: ${scopeIds.length}, (${attr.join('+')})`;
}
22 changes: 15 additions & 7 deletions scopes/workspace/workspace/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,13 @@ export type ComponentExtensionsOpts = {
loadExtensions?: boolean;
};

type ComponentExtensionsResponse = {
extensions: ExtensionDataList;
beforeMerge: Array<{ extensions: ExtensionDataList; origin: ExtensionsOrigin; extraData: any }>; // useful for debugging
errors?: Error[];
envId?: string;
};

export type ExtensionsOrigin =
| 'BitmapFile'
| 'ModelSpecific'
Expand Down Expand Up @@ -1289,20 +1296,21 @@ the following envs are used in this workspace: ${availableEnvs.join(', ')}`);
componentFromScope?: Component,
excludeOrigins: ExtensionsOrigin[] = [],
opts: ComponentExtensionsOpts = {}
): Promise<{
extensions: ExtensionDataList;
beforeMerge: Array<{ extensions: ExtensionDataList; origin: ExtensionsOrigin; extraData: any }>; // useful for debugging
errors?: Error[];
}> {
): Promise<ComponentExtensionsResponse> {
const optsWithDefaults: ComponentExtensionsOpts = Object.assign({ loadExtensions: true }, opts);
const mergeRes = await this.aspectsMerger.merge(componentId, componentFromScope, excludeOrigins);
const mergeRes: ComponentExtensionsResponse = await this.aspectsMerger.merge(
componentId,
componentFromScope,
excludeOrigins
);
const envId = await this.envs.getEnvIdFromEnvsLegacyExtensions(mergeRes.extensions);
if (optsWithDefaults.loadExtensions) {
await this.loadComponentsExtensions(mergeRes.extensions, componentId);
const envId = await this.envs.getEnvIdFromEnvsLegacyExtensions(mergeRes.extensions);
if (envId) {
await this.warnAboutMisconfiguredEnv(envId);
}
}
mergeRes.envId = envId;
return mergeRes;
}

Expand Down