Skip to content

Bound concurrent filesystem reads #279

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.

Sign up for GitHub

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

Open
CR29-22-2805 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Bound concurrent filesystem reads #279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 6 additions & 3 deletions packages/cli/src/util/AssetUploader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { dirExists } from './files.js';
import { retryAsync } from './retryAsync.js';

const DEFAULT_PARALLEL_UPLOADS = 7;
const MAX_PARALLEL_ASSET_READS = 16;

let PARALLEL_UPLOADS = parseInt(process.env['DEVVIT_PARALLEL_UPLOADS'] || '0');
if (isNaN(PARALLEL_UPLOADS) || PARALLEL_UPLOADS < 1) {
Expand Down Expand Up @@ -608,8 +609,9 @@ export async function queryAssets(
const assets = (await tinyglob(assetsGlob, { filesOnly: true, absolute: true })).filter(
(asset) => allowedExtensions.length === 0 || allowedExtensions.includes(path.extname(asset))
);
return await Promise.all(
assets.map(async (asset) => {
return await mapAsyncWithMaxConcurrency(
assets,
async (asset) => {
const filename = path.relative(dir, asset).replaceAll(path.sep, '/');
let file = await fsp.readFile(asset);

Expand All @@ -630,7 +632,8 @@ export async function queryAssets(
isWebviewAsset: assetKind === 'Client',
contents,
};
})
},
MAX_PARALLEL_ASSET_READS
);
}

Expand Down
24 changes: 14 additions & 10 deletions packages/cli/src/util/getAppSourceZip.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import fsp from 'node:fs/promises';
import path from 'node:path';

import { mapAsyncWithMaxConcurrency } from '@devvit/shared-types/mapAsyncWithMaxConcurrency.js';
import ignore from 'ignore';
import JSZip from 'jszip';

import { DevvitCommand } from './commands/DevvitCommand.js';

const ALWAYS_IGNORED_PATHS = Object.freeze(['node_modules', '.env', '.git']);
const MAX_PARALLEL_SOURCE_READS = 16;

export async function getAppSourceZip(cmd: DevvitCommand): Promise<ArrayBuffer> {
const zip = new JSZip();
Expand Down Expand Up @@ -51,7 +53,7 @@ async function addDirectoryToZip(
const entries = await fsp.readdir(dir, { withFileTypes: true });

// Process entries in parallel
const filePromises: Promise<void>[] = [];
const fileCallbacks: (() => Promise<void>)[] = [];
const dirCallbacks: (() => Promise<void>)[] = [];

for (const entry of entries) {
Expand All @@ -71,18 +73,20 @@ async function addDirectoryToZip(
);
}
} else if (entry.isFile()) {
// Read files & add them in parallel though
filePromises.push(
(async () => {
const content = await fsp.readFile(fullPath);
zipFolder.file(entry.name, content);
})()
);
// Defer file reads so concurrency can be bounded below.
fileCallbacks.push(async () => {
const content = await fsp.readFile(fullPath);
zipFolder.file(entry.name, content);
});
}
}

// Wait for all file reads to complete
await Promise.all(filePromises);
// Read files concurrently without opening every file at once.
await mapAsyncWithMaxConcurrency(
fileCallbacks,
(readFile) => readFile(),
MAX_PARALLEL_SOURCE_READS
);

// Then, process the directories in serial
for (const dirCallback of dirCallbacks) {
Expand Down