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

Break up request graph cache serialisation and run after build completion #9384

Merged
merged 24 commits into from
Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
c3fd8f2
Break up request graph cache serialisation and run after build comple…
JakeLane Nov 15, 2023
c06fa33
Fix test to abort before writing to cache
JakeLane Nov 15, 2023
863b5bd
Write cache to disk when build fails
JakeLane Nov 16, 2023
d93e422
Move cache aborting to build queue
JakeLane Nov 16, 2023
63f0017
Fix cache node shallow copy
JakeLane Nov 17, 2023
0cfdedb
Merge branch 'v2' into jlane2/write-request-graph-to-disk-background
JakeLane Nov 28, 2023
d1e726d
Merge branch 'v2' of github.com:parcel-bundler/parcel into jlane2/wri…
JakeLane Dec 8, 2023
b82ab4c
Resolve windows cache path issue
JakeLane Dec 12, 2023
07302ca
Merge branch 'v2' into jlane2/write-request-graph-to-disk-background
JakeLane Dec 12, 2023
80507da
Simplify cache chunking by moving implementation to FSCache
JakeLane Dec 13, 2023
730c274
Use promise queue to manage cache writes
JakeLane Dec 13, 2023
e5d8565
Merge branch 'v2' into jlane2/write-request-graph-to-disk-background
JakeLane Dec 14, 2023
51ffd03
Resolve unhandled errors with promise queue
JakeLane Dec 14, 2023
84d0e1d
Only serialise and write cache chunks to disk when changed
JakeLane Jan 25, 2024
ed110a1
Merge branch 'v2' of github.com:parcel-bundler/parcel into jlane2/wri…
JakeLane Jan 25, 2024
48f1d25
Merge branch 'v2' into jlane2/write-request-graph-to-disk-background
JakeLane Feb 6, 2024
96c1abc
Add getRequestGraphNodeKey function and refactor for loop
JakeLane Feb 8, 2024
c88cab5
Move to set to track cached requests to disk
JakeLane Feb 8, 2024
921c7ba
Bring back catch on queue add
JakeLane Feb 8, 2024
6d339a6
Update unit test for RequestTracker
JakeLane Feb 9, 2024
3965abb
Update progress for cache write and handle node invalidation
JakeLane Feb 15, 2024
e0b3ba2
Update unit test to use new graph set
JakeLane Feb 15, 2024
3d8b1e3
Invalidate written cache on disk on request completion
JakeLane Feb 15, 2024
0941434
Merge branch 'v2' into jlane2/write-request-graph-to-disk-background
JakeLane Feb 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 40 additions & 17 deletions packages/core/cache/src/FSCache.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ import type {FilePath} from '@parcel/types';
import type {FileSystem} from '@parcel/fs';
import type {Cache} from './types';

import type {AbortSignal} from 'abortcontroller-polyfill/dist/cjs-ponyfill';
import stream from 'stream';
import path from 'path';
import {promisify} from 'util';
import logger from '@parcel/logger';
import {serialize, deserialize, registerSerializableClass} from '@parcel/core';
// flowlint-next-line untyped-import:off
import packageJson from '../package.json';

import {WRITE_LIMIT_CHUNK} from './constants';

const pipeline: (Readable, Writable) => Promise<void> = promisify(
Expand Down Expand Up @@ -87,6 +87,15 @@ export class FSCache implements Cache {
return path.join(this.dir, `${key}-${index}`);
}

async #unlinkChunks(key: string, index: number): Promise<void> {
try {
await this.fs.unlink(this.#getFilePath(key, index));
await this.#unlinkChunks(key, index + 1);
} catch (err) {
// If there's an error, no more chunks are left to delete
}
}

hasLargeBlob(key: string): Promise<boolean> {
return this.fs.exists(this.#getFilePath(key, 0));
}
Expand All @@ -102,30 +111,44 @@ export class FSCache implements Cache {
return Buffer.concat(await Promise.all(buffers));
}

async setLargeBlob(key: string, contents: Buffer | string): Promise<void> {
async setLargeBlob(
key: string,
contents: Buffer | string,
options?: {|signal?: AbortSignal|},
): Promise<void> {
const chunks = Math.ceil(contents.length / WRITE_LIMIT_CHUNK);

const writePromises: Promise<void>[] = [];
if (chunks === 1) {
// If there's one chunk, don't slice the content
await this.fs.writeFile(this.#getFilePath(key, 0), contents);
return;
}

const writePromises: Promise<void>[] = [];
for (let i = 0; i < chunks; i += 1) {
writePromises.push(
this.fs.writeFile(
this.#getFilePath(key, i),
typeof contents === 'string'
? contents.slice(i * WRITE_LIMIT_CHUNK, (i + 1) * WRITE_LIMIT_CHUNK)
: contents.subarray(
i * WRITE_LIMIT_CHUNK,
(i + 1) * WRITE_LIMIT_CHUNK,
),
),
this.fs.writeFile(this.#getFilePath(key, 0), contents, {
signal: options?.signal,
}),
);
} else {
for (let i = 0; i < chunks; i += 1) {
writePromises.push(
this.fs.writeFile(
this.#getFilePath(key, i),
typeof contents === 'string'
? contents.slice(
i * WRITE_LIMIT_CHUNK,
(i + 1) * WRITE_LIMIT_CHUNK,
)
: contents.subarray(
i * WRITE_LIMIT_CHUNK,
(i + 1) * WRITE_LIMIT_CHUNK,
),
{signal: options?.signal},
),
);
}
}

// If there's already a files following this chunk, it's old and should be removed
writePromises.push(this.#unlinkChunks(key, chunks));

await Promise.all(writePromises);
}

Expand Down
48 changes: 14 additions & 34 deletions packages/core/cache/src/LMDBCache.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type {FilePath} from '@parcel/types';
import type {Cache} from './types';
import type {Readable, Writable} from 'stream';

import type {AbortSignal} from 'abortcontroller-polyfill/dist/cjs-ponyfill';
import stream from 'stream';
import path from 'path';
import {promisify} from 'util';
Expand All @@ -12,7 +13,8 @@ import {NodeFS} from '@parcel/fs';
import packageJson from '../package.json';
// $FlowFixMe
import lmdb from 'lmdb';
import {WRITE_LIMIT_CHUNK} from './constants';

import {FSCache} from './FSCache';

const pipeline: (Readable, Writable) => Promise<void> = promisify(
stream.pipeline,
Expand All @@ -23,10 +25,12 @@ export class LMDBCache implements Cache {
dir: FilePath;
// $FlowFixMe
store: any;
fsCache: FSCache;

constructor(cacheDir: FilePath) {
this.fs = new NodeFS();
this.dir = cacheDir;
this.fsCache = new FSCache(this.fs, cacheDir);

this.store = lmdb.open(cacheDir, {
name: 'parcel-cache',
Expand Down Expand Up @@ -100,42 +104,18 @@ export class LMDBCache implements Cache {
return this.fs.exists(this.#getFilePath(key, 0));
}

// eslint-disable-next-line require-await
async getLargeBlob(key: string): Promise<Buffer> {
const buffers: Promise<Buffer>[] = [];
for (let i = 0; await this.fs.exists(this.#getFilePath(key, i)); i += 1) {
const file: Promise<Buffer> = this.fs.readFile(this.#getFilePath(key, i));

buffers.push(file);
}

return Buffer.concat(await Promise.all(buffers));
return this.fsCache.getLargeBlob(key);
}

async setLargeBlob(key: string, contents: Buffer | string): Promise<void> {
const chunks = Math.ceil(contents.length / WRITE_LIMIT_CHUNK);

if (chunks === 1) {
// If there's one chunk, don't slice the content
await this.fs.writeFile(this.#getFilePath(key, 0), contents);
return;
}

const writePromises: Promise<void>[] = [];
for (let i = 0; i < chunks; i += 1) {
writePromises.push(
this.fs.writeFile(
this.#getFilePath(key, i),
typeof contents === 'string'
? contents.slice(i * WRITE_LIMIT_CHUNK, (i + 1) * WRITE_LIMIT_CHUNK)
: contents.subarray(
i * WRITE_LIMIT_CHUNK,
(i + 1) * WRITE_LIMIT_CHUNK,
),
),
);
}

await Promise.all(writePromises);
// eslint-disable-next-line require-await
async setLargeBlob(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any elegant way to share the implementation between the two Cache types as (AFAICT) they're identical?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instantiate a FSCache instance inside lmdb cache and forward calls of setLargeBlob to that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds good to me, will clean this up

key: string,
contents: Buffer | string,
options?: {|signal?: AbortSignal|},
): Promise<void> {
return this.fsCache.setLargeBlob(key, contents, options);
}

refresh(): void {
Expand Down
7 changes: 6 additions & 1 deletion packages/core/cache/src/types.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// @flow
import type {Readable} from 'stream';
import type {AbortSignal} from 'abortcontroller-polyfill/dist/cjs-ponyfill';

export interface Cache {
ensure(): Promise<void>;
Expand All @@ -12,7 +13,11 @@ export interface Cache {
setBlob(key: string, contents: Buffer | string): Promise<void>;
hasLargeBlob(key: string): Promise<boolean>;
getLargeBlob(key: string): Promise<Buffer>;
setLargeBlob(key: string, contents: Buffer | string): Promise<void>;
setLargeBlob(
key: string,
contents: Buffer | string,
options?: {|signal?: AbortSignal|},
): Promise<void>;
getBuffer(key: string): Promise<?Buffer>;
/**
* In a multi-threaded environment, where there are potentially multiple Cache
Expand Down
29 changes: 27 additions & 2 deletions packages/core/core/src/Parcel.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ export default class Parcel {
}

let result = await this._build({startTime});

await this.#requestTracker.writeToCache();
await this._end();

if (result.type === 'buildFailure') {
Expand All @@ -175,10 +177,31 @@ export default class Parcel {
async _end(): Promise<void> {
this.#initialized = false;

await this.#requestTracker.writeToCache();
await this.#disposable.dispose();
}

async writeRequestTrackerToCache(): Promise<void> {
if (this.#watchQueue.getNumWaiting() === 0) {
// If there's no queued events, we are safe to write the request graph to disk
const abortController = new AbortController();

const unsubscribe = this.#watchQueue.subscribeToAdd(() => {
abortController.abort();
});

try {
await this.#requestTracker.writeToCache(abortController.signal);
} catch (err) {
if (!abortController.signal.aborted) {
// We expect abort errors if we interrupt the cache write
throw err;
}
}

unsubscribe();
}
}

async _startNextBuild(): Promise<?BuildEvent> {
this.#watchAbortController = new AbortController();
await this.#farm.callAllWorkers('clearConfigCache', []);
Expand All @@ -198,6 +221,9 @@ export default class Parcel {
if (!(err instanceof BuildAbortError)) {
throw err;
}
} finally {
// If the build passes or fails, we want to cache the request graph
await this.writeRequestTrackerToCache();
}
}

Expand Down Expand Up @@ -372,7 +398,6 @@ export default class Parcel {
};

await this.#reporterRunner.report(event);

return event;
} finally {
if (this.isProfiling) {
Expand Down