Skip to content

Commit 262744e

Browse files
authored
Merge pull request #3155 from github/koesie10/yauzl-concurrent
Add concurrency to unzipping using `yauzl`
2 parents e824fda + 4444951 commit 262744e

File tree

7 files changed

+111
-36
lines changed

7 files changed

+111
-36
lines changed

extensions/ql-vscode/scripts/source-map.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { spawnSync } from "child_process";
2020
import { basename, resolve } from "path";
2121
import { pathExists, readJSON } from "fs-extra";
2222
import { RawSourceMap, SourceMapConsumer } from "source-map";
23-
import { unzipToDirectory } from "../src/common/unzip";
23+
import { unzipToDirectorySequentially } from "../src/common/unzip";
2424

2525
if (process.argv.length !== 4) {
2626
console.error(
@@ -78,7 +78,7 @@ async function extractSourceMap() {
7878
releaseAssetsDirectory,
7979
]);
8080

81-
await unzipToDirectory(
81+
await unzipToDirectorySequentially(
8282
resolve(releaseAssetsDirectory, sourcemapAsset.name),
8383
sourceMapsDirectory,
8484
);

extensions/ql-vscode/src/codeql-cli/distribution.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import {
2525
showAndLogErrorMessage,
2626
showAndLogWarningMessage,
2727
} from "../common/logging";
28-
import { unzipToDirectory } from "../common/unzip";
28+
import { unzipToDirectoryConcurrently } from "../common/unzip-concurrently";
2929

3030
/**
3131
* distribution.ts
@@ -420,7 +420,10 @@ class ExtensionSpecificDistributionManager {
420420
void extLogger.log(
421421
`Extracting CodeQL CLI to ${this.getDistributionStoragePath()}`,
422422
);
423-
await unzipToDirectory(archivePath, this.getDistributionStoragePath());
423+
await unzipToDirectoryConcurrently(
424+
archivePath,
425+
this.getDistributionStoragePath(),
426+
);
424427
} finally {
425428
await remove(tmpDirectory);
426429
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { availableParallelism } from "os";
2+
import { unzipToDirectory } from "./unzip";
3+
import PQueue from "p-queue";
4+
5+
export async function unzipToDirectoryConcurrently(
6+
archivePath: string,
7+
destinationPath: string,
8+
): Promise<void> {
9+
const queue = new PQueue({
10+
concurrency: availableParallelism(),
11+
});
12+
13+
return unzipToDirectory(archivePath, destinationPath, async (tasks) => {
14+
await queue.addAll(tasks);
15+
});
16+
}

extensions/ql-vscode/src/common/unzip.ts

Lines changed: 70 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,57 @@ async function copyStream(
9797
});
9898
}
9999

100+
/**
101+
* Unzips a single file from a zip archive.
102+
*
103+
* @param zipFile
104+
* @param entry
105+
* @param rootDestinationPath
106+
*/
107+
async function unzipFile(
108+
zipFile: ZipFile,
109+
entry: ZipEntry,
110+
rootDestinationPath: string,
111+
): Promise<void> {
112+
const path = join(rootDestinationPath, entry.fileName);
113+
114+
if (/\/$/.test(entry.fileName)) {
115+
// Directory file names end with '/'
116+
117+
await ensureDir(path);
118+
} else {
119+
// Ensure the directory exists
120+
await ensureDir(dirname(path));
121+
122+
const readable = await openZipReadStream(zipFile, entry);
123+
124+
let mode: number | undefined = entry.externalFileAttributes >>> 16;
125+
if (mode <= 0) {
126+
mode = undefined;
127+
}
128+
129+
const writeStream = createWriteStream(path, {
130+
autoClose: true,
131+
mode,
132+
});
133+
134+
await copyStream(readable, writeStream);
135+
}
136+
}
137+
138+
/**
139+
* Unzips all files from a zip archive. Please use
140+
* `unzipToDirectoryConcurrently` or `unzipToDirectorySequentially` instead
141+
* of this function.
142+
*
143+
* @param archivePath
144+
* @param destinationPath
145+
* @param taskRunner A function that runs the tasks (either sequentially or concurrently).
146+
*/
100147
export async function unzipToDirectory(
101148
archivePath: string,
102149
destinationPath: string,
150+
taskRunner: (tasks: Array<() => Promise<void>>) => Promise<void>,
103151
): Promise<void> {
104152
const zipFile = await openZip(archivePath, {
105153
autoClose: false,
@@ -110,33 +158,29 @@ export async function unzipToDirectory(
110158
try {
111159
const entries = await readZipEntries(zipFile);
112160

113-
for (const entry of entries) {
114-
const path = join(destinationPath, entry.fileName);
115-
116-
if (/\/$/.test(entry.fileName)) {
117-
// Directory file names end with '/'
118-
119-
await ensureDir(path);
120-
} else {
121-
// Ensure the directory exists
122-
await ensureDir(dirname(path));
123-
124-
const readable = await openZipReadStream(zipFile, entry);
125-
126-
let mode: number | undefined = entry.externalFileAttributes >>> 16;
127-
if (mode <= 0) {
128-
mode = undefined;
129-
}
130-
131-
const writeStream = createWriteStream(path, {
132-
autoClose: true,
133-
mode,
134-
});
135-
136-
await copyStream(readable, writeStream);
137-
}
138-
}
161+
await taskRunner(
162+
entries.map((entry) => () => unzipFile(zipFile, entry, destinationPath)),
163+
);
139164
} finally {
140165
zipFile.close();
141166
}
142167
}
168+
169+
/**
170+
* Sequentially unzips all files from a zip archive. Please use
171+
* `unzipToDirectoryConcurrently` if you can. This function is only
172+
* provided because Jest cannot import `p-queue`.
173+
*
174+
* @param archivePath
175+
* @param destinationPath
176+
*/
177+
export async function unzipToDirectorySequentially(
178+
archivePath: string,
179+
destinationPath: string,
180+
): Promise<void> {
181+
return unzipToDirectory(archivePath, destinationPath, async (tasks) => {
182+
for (const task of tasks) {
183+
await task();
184+
}
185+
});
186+
}

extensions/ql-vscode/src/variant-analysis/variant-analysis-results-manager.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
} from "./shared/variant-analysis";
1717
import { DisposableObject, DisposeHandler } from "../common/disposable-object";
1818
import { EventEmitter } from "vscode";
19-
import { unzipToDirectory } from "../common/unzip";
19+
import { unzipToDirectoryConcurrently } from "../common/unzip-concurrently";
2020
import { readRepoTask, writeRepoTask } from "./repo-tasks-store";
2121

2222
type CacheKey = `${number}/${string}`;
@@ -106,7 +106,7 @@ export class VariantAnalysisResultsManager extends DisposableObject {
106106
VariantAnalysisResultsManager.RESULTS_DIRECTORY,
107107
);
108108

109-
await unzipToDirectory(zipFilePath, unzippedFilesDirectory);
109+
await unzipToDirectoryConcurrently(zipFilePath, unzippedFilesDirectory);
110110

111111
this._onResultDownloaded.fire({
112112
variantAnalysisId,

extensions/ql-vscode/test/unit-tests/common/unzip.test.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@ import {
88
openZip,
99
openZipBuffer,
1010
readZipEntries,
11-
unzipToDirectory,
11+
unzipToDirectorySequentially,
1212
} from "../../../src/common/unzip";
1313
import { walkDirectory } from "../../../src/common/files";
14+
import { unzipToDirectoryConcurrently } from "../../../src/common/unzip-concurrently";
1415

1516
const zipPath = resolve(__dirname, "../data/unzip/test-zip.zip");
1617

@@ -88,7 +89,16 @@ describe("openZipBuffer", () => {
8889
});
8990
});
9091

91-
describe("unzipToDirectory", () => {
92+
describe.each([
93+
{
94+
name: "unzipToDirectorySequentially",
95+
unzipToDirectory: unzipToDirectorySequentially,
96+
},
97+
{
98+
name: "unzipToDirectoryConcurrently",
99+
unzipToDirectory: unzipToDirectoryConcurrently,
100+
},
101+
])("$name", ({ unzipToDirectory }) => {
92102
let tmpDir: DirectoryResult;
93103

94104
beforeEach(async () => {
@@ -186,6 +196,8 @@ async function expectFile(
186196
if (expectedContents) {
187197
expect(contents.toString("utf-8")).toEqual(expectedContents);
188198
}
199+
200+
await file.close();
189201
}
190202

191203
async function computeHash(contents: Buffer) {

extensions/ql-vscode/test/vscode-tests/ensureCli.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {
44
getRequiredAssetName,
55
codeQlLauncherName,
66
} from "../../src/common/distribution";
7-
import { unzipToDirectory } from "../../src/common/unzip";
7+
import { unzipToDirectorySequentially } from "../../src/common/unzip";
88
import fetch from "node-fetch";
99
import supportedCliVersions from "../../supported_cli_versions.json";
1010

@@ -126,7 +126,7 @@ export async function ensureCli(useCli: boolean) {
126126

127127
console.log(`Unzipping into '${unzipDir}'`);
128128
mkdirpSync(unzipDir);
129-
await unzipToDirectory(downloadedFilePath, unzipDir);
129+
await unzipToDirectorySequentially(downloadedFilePath, unzipDir);
130130
console.log("Done.");
131131
} catch (e) {
132132
console.error("Failed to download CLI.");

0 commit comments

Comments
 (0)