Skip to content

Commit

Permalink
perf(install/link) - prevent clear cache in many cases during bit ins…
Browse files Browse the repository at this point in the history 
…tall/link (#8939)

Co-authored-by: David First <[email protected]>
  • Loading branch information
@GiladShoham @davidfirst
GiladShoham and davidfirst authored Jun 5, 2024
1 parent 2df18a3 commit 8239050
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 47 deletions.
8 changes: 4 additions & 4 deletions scopes/compilation/compiler/workspace-compiler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,6 @@ export class ComponentCompiler {
dataToPersist.addManyFiles(this.dists);
dataToPersist.addBasePath(this.workspace.path);
await dataToPersist.persistAllToFS();
const linkComponents = options.linkComponents ?? true;
if (linkComponents) {
await linkToNodeModulesByComponents([this.component], this.workspace);
}
const buildResults = this.dists.map((distFile) => distFile.path);
if (this.component.state._consumer.compiler) loader.succeed();

Expand Down Expand Up @@ -338,6 +334,10 @@ export class WorkspaceCompiler {
const grouped = this.groupByIsEnv(components);
const envsResults = grouped.envs ? await this.runCompileComponents(grouped.envs, options, noThrow) : [];
const otherResults = grouped.other ? await this.runCompileComponents(grouped.other, options, noThrow) : [];
const linkComponents = options.linkComponents ?? true;
if (linkComponents) {
await linkToNodeModulesByComponents(components, this.workspace);
}
return [...envsResults, ...otherResults];
}

Expand Down
31 changes: 21 additions & 10 deletions scopes/workspace/install/install.main.runtime.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,21 +358,23 @@ export class InstallMain {
);
let cacheCleared = false;
await this.linkCodemods(compDirMap);
const shouldClearCacheOnInstall = this.shouldClearCacheOnInstall();
if (options?.compile ?? true) {
const compileStartTime = process.hrtime();
const compileOutputMessage = `compiling components`;
this.logger.setStatusLine(compileOutputMessage);
// We need to clear cache before compiling the components or it might compile them with the default env
// incorrectly in case the env was not loaded correctly before the install
this.workspace.consumer.componentLoader.clearComponentsCache();
// We don't want to clear the failed to load envs because we want to show the warning at the end
await this.workspace.clearCache({ skipClearFailedToLoadEnvs: true });
cacheCleared = true;
if (shouldClearCacheOnInstall) {
// We need to clear cache before compiling the components or it might compile them with the default env
// incorrectly in case the env was not loaded correctly before the installation.
// We don't want to clear the failed to load envs because we want to show the warning at the end
await this.workspace.clearCache({ skipClearFailedToLoadEnvs: true });
cacheCleared = true;
}
await this.compiler.compileOnWorkspace([], { initiator: CompilationInitiator.Install });
this.logger.consoleSuccess(compileOutputMessage, compileStartTime);
}
if (options?.writeConfigFiles ?? true) {
await this.tryWriteConfigFiles(!cacheCleared);
await this.tryWriteConfigFiles(!cacheCleared && shouldClearCacheOnInstall);
}
if (!dependenciesChanged) break;
if (!options?.recurringInstall) break;
Expand All @@ -381,9 +383,9 @@ export class InstallMain {
prevManifests.add(manifestsHash(current.manifests));
// If we run compile we do the clear cache before the compilation so no need to clean it again (it's an expensive
// operation)
if (!cacheCleared) {
if (!cacheCleared && shouldClearCacheOnInstall) {
// We need to clear cache before creating the new component manifests.
this.workspace.consumer.componentLoader.clearComponentsCache();
// this.workspace.consumer.componentLoader.clearComponentsCache();
// We don't want to clear the failed to load envs because we want to show the warning at the end
await this.workspace.clearCache({ skipClearFailedToLoadEnvs: true });
}
Expand All @@ -395,11 +397,20 @@ export class InstallMain {
// Otherwise, we might load an env from a location that we later remove.
await installer.pruneModules(this.workspace.path);
}
await this.workspace.consumer.componentFsCache.deleteAllDependenciesDataCache();
// this is now commented out because we assume we don't need it anymore.
// even when the env was not loaded before and it is loaded now, it should be fine because the dependencies-data
// is only about the auto-detect-deps. there are two more steps: version-resolution and apply-overrides that
// disregard the dependencies-cache.
// await this.workspace.consumer.componentFsCache.deleteAllDependenciesDataCache();
/* eslint-enable no-await-in-loop */
return current.componentDirectoryMap;
}

private shouldClearCacheOnInstall(): boolean {
const nonLoadedEnvs = this.envs.getFailedToLoadEnvs();
return nonLoadedEnvs.length > 0;
}

private async _getComponentsManifestsAndRootPolicy(
installer: DependencyInstaller,
options: GetComponentsAndManifestsOptions & {
Expand Down
96 changes: 68 additions & 28 deletions scopes/workspace/modules/node-modules-linker/node-modules-linker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { Component } from '@teambit/component';
import { PackageJsonTransformer } from './package-json-transformer';
import { changeCodeFromRelativeToModulePaths } from './codemod-components';

type LinkDetail = { from: string; to: string };
type LinkDetail = { from: string; to: string; existsBefore: boolean };
export type NodeModulesLinksResult = {
id: ComponentID;
bound: LinkDetail[];
Expand All @@ -35,31 +35,42 @@ export default class NodeModuleLinker {
consumer: Consumer;
bitMap: BitMap; // preparation for the capsule, which is going to have only BitMap with no Consumer
dataToPersist: DataToPersist;
existingLinks: NodeModulesLinksResult[];
packageJsonCreated: boolean;

constructor(private components: Component[], private workspace: Workspace) {
this.consumer = this.workspace.consumer;
this.bitMap = this.consumer.bitMap;
this.dataToPersist = new DataToPersist();
this.existingLinks = [];
this.packageJsonCreated = false;
}
async link(): Promise<NodeModulesLinksResult[]> {
this.components = this.components.filter((component) => this.bitMap.getComponentIfExist(component.id));
const links = await this.getLinks();

const linksResults = this.getLinksResults();
if (!linksResults.length) {
// avoid clearing the cache if it ends up with no links. (e.g. happens when mistakenly generating links for a
// component not in the workspace)
// or when all links are already exist.
return [];
}
const workspacePath = this.workspace.path;
links.addBasePath(workspacePath);
await links.persistAllToFS();
await Promise.all(
this.components.map((component) =>
this.consumer?.componentFsCache.deleteDependenciesDataCache(component.id.toString())
)
);
// if this cache is not cleared, then when asking workspace.get again to the same component, it returns it with
// component-issues like "MissingLinksFromNodeModulesToSrc" incorrectly.
this.workspace.clearAllComponentsCache();
// Only clear cache if new package.json of components were created
if (this.packageJsonCreated) {
await Promise.all(
this.components.map((component) =>
this.consumer?.componentFsCache.deleteDependenciesDataCache(component.id.toString())
)
);
// if this cache is not cleared, then when asking workspace.get again to the same component, it returns it with
// component-issues like "MissingLinksFromNodeModulesToSrc" incorrectly.
this.workspace.clearAllComponentsCache();
}

await linkPkgsToBitRoots(
workspacePath,
this.components.map((comp) => componentIdToPackageName(comp.state._consumer))
Expand All @@ -76,23 +87,35 @@ export default class NodeModuleLinker {

return this.dataToPersist;
}
private addLinkResult(
linksResults: NodeModulesLinksResult[],
id: ComponentID | null | undefined,
from: string,
to: string,
existsBefore: boolean
) {
if (!id) return;
const existingLinkResult = linksResults.find((linkResult) => linkResult.id.isEqual(id));
if (existingLinkResult) {
existingLinkResult.bound.push({ from, to, existsBefore });
} else {
linksResults.push({ id, bound: [{ from, to, existsBefore }] });
}
}

getLinksResults(): NodeModulesLinksResult[] {
const linksResults: NodeModulesLinksResult[] = [];
const getExistingLinkResult = (id: ComponentID) => linksResults.find((linkResult) => linkResult.id.isEqual(id));
const addLinkResult = (id: ComponentID | null | undefined, from: string, to: string) => {
if (!id) return;
const existingLinkResult = getExistingLinkResult(id);
if (existingLinkResult) {
existingLinkResult.bound.push({ from, to });
} else {
linksResults.push({ id, bound: [{ from, to }] });
}
};
this.dataToPersist.symlinks.forEach((symlink: Symlink) => {
addLinkResult(symlink.componentId, symlink.src, symlink.dest);
this.addLinkResult(linksResults, symlink.componentId, symlink.src, symlink.dest, false);
});
this.existingLinks.forEach((link: NodeModulesLinksResult) => {
const componentId = link.id;
link.bound.forEach((bound) => {
this.addLinkResult(linksResults, componentId, bound.from, bound.to, true);
});
});
this.components.forEach((component) => {
const existingLinkResult = getExistingLinkResult(component.id);
const existingLinkResult = linksResults.find((linkResult) => linkResult.id.isEqual(component.id));
if (!existingLinkResult) {
linksResults.push({ id: component.id, bound: [] });
}
Expand All @@ -117,20 +140,32 @@ export default class NodeModuleLinker {
const legacyComponent = component.state._consumer as ConsumerComponent;
const linkPath: PathOsBasedRelative = getNodeModulesPathOfComponent(legacyComponent);

this.symlinkComponentDir(component, linkPath);
await this.symlinkComponentDir(component, linkPath);
this._deleteExistingLinksRootIfSymlink(linkPath);
await this.createPackageJson(component);
}

private symlinkComponentDir(component: Component, linkPath: PathOsBasedRelative) {
private async symlinkComponentDir(component: Component, linkPath: PathOsBasedRelative) {
const componentMap = this.bitMap.getComponent(component.id);

const filesToBind = componentMap.getAllFilesPaths();
filesToBind.forEach((file) => {
const fileWithRootDir = path.join(componentMap.rootDir as string, file);
const dest = path.join(linkPath, file);
this.dataToPersist.addSymlink(Symlink.makeInstance(fileWithRootDir, dest, component.id, true));
});
await Promise.all(
filesToBind.map(async (file) => {
const fileWithRootDir = path.join(componentMap.rootDir as string, file);
const dest = path.join(linkPath, file);
let stat;
try {
stat = await fs.lstat(dest);
} catch (err) {
// Ignore this error, it's probably because the file doesn't exist
}
if (stat && stat.isSymbolicLink()) {
this.addLinkResult(this.existingLinks, component.id, fileWithRootDir, dest, true);
} else {
this.dataToPersist.addSymlink(Symlink.makeInstance(fileWithRootDir, dest, component.id, true));
}
})
);

if (IS_WINDOWS) {
// symlink the entire source directory into "_src" in node-modules.
Expand Down Expand Up @@ -180,6 +215,11 @@ export default class NodeModuleLinker {
id: legacyComp.id,
})
);
const packageJsonExist = await fs.pathExists(path.join(dest, 'package.json'));
if (!packageJsonExist) {
this.packageJsonCreated = true;
}

const packageJson = PackageJsonFile.createFromComponent(dest, legacyComp, true);
await this._applyTransformers(component, packageJson);
if (IS_WINDOWS) {
Expand Down
14 changes: 10 additions & 4 deletions scopes/workspace/workspace/workspace.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -741,17 +741,23 @@ it's possible that the version ${component.id.version} belong to ${idStr.split('
return workspaceAspectsLoader.getConfiguredUserAspectsPackages(options);
}

/**
* clears workspace, scope and all components caches.
* doesn't clear the dependencies-data from the filesystem-cache.
*/
async clearCache(options: ClearCacheOptions = {}) {
this.logger.debug('clearing the workspace and scope caches');
this.aspectLoader.resetFailedLoadAspects();
if (!options.skipClearFailedToLoadEnvs) this.envs.resetFailedToLoadEnvs();
this.logger.debug('clearing the workspace and scope caches');
this.componentLoader.clearCache();
this.componentStatusLoader.clearCache();
await this.scope.clearCache();
this.componentList = new ComponentsList(this.consumer);
this.componentDefaultScopeFromComponentDirAndNameWithoutConfigFileMemoized.clear();
this.clearAllComponentsCache();
}

/**
* clear the cache of all components in the workspace.
* doesn't clear the dependencies-data from the filesystem-cache.
*/
clearAllComponentsCache() {
this.logger.debug('clearing all components caches');
this.componentLoader.clearCache();
Expand Down
4 changes: 3 additions & 1 deletion src/consumer/component/component-loader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ export default class ComponentLoader {
const pathsToCheck = [
path.join(this.consumer.getPath(), 'node_modules'),
path.join(this.consumer.getPath(), 'package.json'),
path.join(this.consumer.getPath(), 'pnpm-lock.yaml'),
path.join(this.consumer.getPath(), 'yarn.lock'),
path.join(this.consumer.getPath(), BIT_MAP),
this.consumer.config.path,
];
Expand Down Expand Up @@ -281,10 +283,10 @@ export default class ComponentLoader {
* this function returns true only if the dependencies cache has all the components. or when only one component is missing.
*/
private async shouldRunInParallel(ids: ComponentID[]): Promise<boolean> {
await this.invalidateDependenciesCacheIfNeeded();
if (ids.length < 2) {
return false;
}
await this.invalidateDependenciesCacheIfNeeded();
const dependenciesCacheList = await this.componentFsCache.listDependenciesDataCache();
const depsInCache = Object.keys(dependenciesCacheList);
if (!depsInCache.length) {
Expand Down

0 comments on commit 8239050

Please sign in to comment.