From 1c0f94a72042f1798fcdccbb99caa50ed9770ba5 Mon Sep 17 00:00:00 2001 From: Yuki Hattori Date: Tue, 14 Jan 2025 18:58:54 +0900 Subject: [PATCH 1/7] Allow parallel conversion in `Converter.convertFiles()` --- src/browser/browser.ts | 59 +++++++++++++-------- src/browser/manager.ts | 44 ++++++++-------- src/converter.ts | 15 +++++- src/utils/memoized-promise.ts | 20 +++++++ src/utils/wsl.ts | 98 +++++++++++++++-------------------- 5 files changed, 135 insertions(+), 101 deletions(-) create mode 100644 src/utils/memoized-promise.ts diff --git a/src/browser/browser.ts b/src/browser/browser.ts index 251660cc..f222707f 100644 --- a/src/browser/browser.ts +++ b/src/browser/browser.ts @@ -2,6 +2,7 @@ import { EventEmitter } from 'node:events' import fs from 'node:fs' import os from 'node:os' import path from 'node:path' +import { isPromise } from 'node:util/types' import { nanoid } from 'nanoid' import type { Browser as PuppeteerBrowser, @@ -11,6 +12,7 @@ import type { } from 'puppeteer-core' import type TypedEventEmitter from 'typed-emitter' import { debugBrowser } from '../utils/debug' +import { createMemoizedPromiseContext } from '../utils/memoized-promise' import { getWindowsEnv, isWSL, @@ -46,11 +48,11 @@ export abstract class Browser path: string protocolTimeout: number - puppeteer: PuppeteerBrowser | undefined timeout: number #dataDirName: string - private _puppeteerDataDir?: string + private _puppeteerDataDir = createMemoizedPromiseContext() + private _puppeteer = createMemoizedPromiseContext() constructor(opts: BrowserOptions) { super() @@ -70,29 +72,32 @@ export abstract class Browser return (this.constructor as typeof Browser).protocol } - async launch(opts: LaunchOptions = {}): Promise { - if (!this.puppeteer) { + async launch(opts: LaunchOptions = {}) { + return this._puppeteer.init(async () => { + debugBrowser('Launching browser via Puppeteer...') + const puppeteer = await this.launchPuppeteer(opts) puppeteer.once('disconnected', () => { this.emit('disconnect', puppeteer) - this.puppeteer = undefined + this._puppeteer.value = undefined debugBrowser('Browser disconnected (Cleaned up puppeteer instance)') }) - this.puppeteer = puppeteer this.emit('launch', puppeteer) return puppeteer - } - return this.puppeteer + }) } async withPage(fn: (page: Page) => T) { + const debugPageId = nanoid(8) const puppeteer = await this.launch() const page = await puppeteer.newPage() + debugBrowser('Created a new page [%s]', debugPageId) + page.setDefaultTimeout(this.timeout) page.setDefaultNavigationTimeout(this.timeout) @@ -100,19 +105,20 @@ export abstract class Browser return await fn(page) } finally { await page.close() + debugBrowser('Page closed [%s]', debugPageId) } } async close() { - if (this.puppeteer) { - const { puppeteer } = this + const pptr = await this._puppeteer.value - if (puppeteer.connected) { - await puppeteer.close() - this.emit('close', puppeteer) + if (pptr) { + if (pptr.connected) { + await pptr.close() + this.emit('close', pptr) } - this.puppeteer = undefined + this._puppeteer.value = undefined } } @@ -123,7 +129,15 @@ export abstract class Browser async browserInWSLHost(): Promise { return ( !!(await isWSL()) && - wslHostMatcher.test(this.puppeteer?.process()?.spawnfile ?? this.path) + wslHostMatcher.test( + // This function may be called while launching Puppeteer. If the browser + // value awaited, Marp CLI will bring deadlock. So we should check the + // value is already resolved (non-Promise truthy value) or not (Promise + // or undefined). + (this._puppeteer.value && !isPromise(this._puppeteer.value) + ? this._puppeteer.value.process()?.spawnfile + : null) ?? this.path + ) ) } @@ -160,10 +174,10 @@ export abstract class Browser /** @internal */ protected async puppeteerDataDir() { - if (this._puppeteerDataDir === undefined) { + return this._puppeteerDataDir.init(async () => { let needToTranslateWindowsPathToWSL = false - this._puppeteerDataDir = await (async () => { + const dir = await (async () => { // In WSL environment, Marp CLI may use Chrome on Windows. If Chrome has // located in host OS (Windows), we have to specify Windows path. if (await this.browserInWSLHost()) { @@ -176,16 +190,15 @@ export abstract class Browser return path.resolve(os.tmpdir(), this.#dataDirName) })() - debugBrowser(`Chrome data directory: %s`, this._puppeteerDataDir) - // Ensure the data directory is created const mkdirPath = needToTranslateWindowsPathToWSL - ? await translateWindowsPathToWSL(this._puppeteerDataDir) - : this._puppeteerDataDir + ? await translateWindowsPathToWSL(dir) + : dir await fs.promises.mkdir(mkdirPath, { recursive: true }) debugBrowser(`Created data directory: %s`, mkdirPath) - } - return this._puppeteerDataDir + + return dir + }) } } diff --git a/src/browser/manager.ts b/src/browser/manager.ts index da806691..1a00c11c 100644 --- a/src/browser/manager.ts +++ b/src/browser/manager.ts @@ -1,5 +1,6 @@ import { error } from '../error' import { debugBrowser } from '../utils/debug' +import { createMemoizedPromiseContext } from '../utils/memoized-promise' import type { Browser, BrowserProtocol } from './browser' import { ChromeCdpBrowser } from './browsers/chrome-cdp' import { defaultFinders, findBrowser } from './finder' @@ -23,12 +24,12 @@ export class BrowserManager implements AsyncDisposable { // Finder private _finders: readonly FinderName[] = defaultFinders private _finderPreferredPath?: string - private _finderResult?: BrowserFinderResult + private _finderResult = createMemoizedPromiseContext() // Browser - private _conversionBrowser?: Browser + private _conversionBrowser = createMemoizedPromiseContext() private _preferredProtocol: BrowserProtocol = 'webDriverBiDi' - private _previewBrowser?: ChromeCdpBrowser + private _previewBrowser = createMemoizedPromiseContext() private _timeout?: number constructor(config: BrowserManagerConfig = {}) { @@ -42,11 +43,11 @@ export class BrowserManager implements AsyncDisposable { configure(config: BrowserManagerConfig) { if (config.finders) { this._finders = ([] as FinderName[]).concat(config.finders) - this._finderResult = undefined // Reset finder result cache + this._finderResult.value = undefined // Reset finder result cache } if (config.path !== undefined) { this._finderPreferredPath = config.path - this._finderResult = undefined // Reset finder result cache + this._finderResult.value = undefined // Reset finder result cache } if (config.protocol) { if (this._conversionBrowser) @@ -62,14 +63,14 @@ export class BrowserManager implements AsyncDisposable { } async findBrowser() { - return (this._finderResult ??= await findBrowser(this._finders, { - preferredPath: this._finderPreferredPath, - })) + return this._finderResult.init(() => + findBrowser(this._finders, { preferredPath: this._finderPreferredPath }) + ) } // Browser for converter async browserForConversion(): Promise { - if (!this._conversionBrowser) { + return this._conversionBrowser.init(async () => { const { acceptedBrowsers, path } = await this.findBrowser() const browser = @@ -90,14 +91,13 @@ export class BrowserManager implements AsyncDisposable { debugBrowser('Use browser class for conversion: %o', browser) // @ts-expect-error ts2511: TS cannot create an instance of an abstract class - this._conversionBrowser = new browser({ path, timeout: this.timeout }) - } - return this._conversionBrowser! + return new browser({ path, timeout: this.timeout }) + }) } // Browser for preview window async browserForPreview(): Promise { - if (!this._previewBrowser) { + return this._previewBrowser.init(async () => { const { acceptedBrowsers, path } = await this.findBrowser() if (!acceptedBrowsers.some((browser) => browser === ChromeCdpBrowser)) { @@ -105,18 +105,20 @@ export class BrowserManager implements AsyncDisposable { } debugBrowser('Use browser class for preview: %o', ChromeCdpBrowser) - this._previewBrowser = new ChromeCdpBrowser({ - path, - timeout: this.timeout, - }) - } - return this._previewBrowser + return new ChromeCdpBrowser({ path, timeout: this.timeout }) + }) } async dispose() { await Promise.all([ - this._conversionBrowser?.close(), - this._previewBrowser?.close(), + async () => { + await (await this._conversionBrowser.value)?.close() + this._conversionBrowser.value = undefined + }, + async () => { + await (await this._previewBrowser.value)?.close() + this._previewBrowser.value = undefined + }, ]) } diff --git a/src/converter.ts b/src/converter.ts index c7b78069..1e857c80 100644 --- a/src/converter.ts +++ b/src/converter.ts @@ -74,6 +74,7 @@ export interface ConverterOption { options: MarpitOptions output?: string | false pages?: boolean | number[] + parallel?: number pdfNotes?: boolean pdfOutlines?: | false @@ -211,6 +212,8 @@ export class Converter { } if (!opts.onlyScanning) { + debug('Converting %s ...', file.relativePath()) + const files: File[] = [] switch (this.options.type) { case ConvertType.pdf: @@ -267,7 +270,17 @@ export class Converter { if (!inputDir && output && output !== '-' && files.length > 1) error('Output path cannot specify with processing multiple files.') - for (const file of files) await this.convertFile(file, opts) + const parallel = Math.min(1, this.options.parallel ?? 1) + const queue = [...files] + + const workers = Array.from({ length: parallel }, async () => { + while (queue.length > 0) { + const file = queue.shift() + if (file) await this.convertFile(file, opts) + } + }) + + await Promise.all(workers) } private convertFileToHTML(tpl: TemplateResult, file: File): File { diff --git a/src/utils/memoized-promise.ts b/src/utils/memoized-promise.ts new file mode 100644 index 00000000..ee82a585 --- /dev/null +++ b/src/utils/memoized-promise.ts @@ -0,0 +1,20 @@ +type MemoizedPromiseAllowedValue = NonNullable | null + +export interface MemoizedPromiseContext { + value: Promise | T | undefined + init: (initializer: () => T | Promise) => Promise +} + +export const createMemoizedPromiseContext = < + T extends MemoizedPromiseAllowedValue, +>(): MemoizedPromiseContext => { + const ctx: MemoizedPromiseContext = { + value: undefined, + init: async (initializer) => + await (ctx.value ??= Promise.resolve(initializer()).then( + (v) => (ctx.value = v) + )), + } + + return ctx +} diff --git a/src/utils/wsl.ts b/src/utils/wsl.ts index 676b3268..10f046fa 100644 --- a/src/utils/wsl.ts +++ b/src/utils/wsl.ts @@ -2,6 +2,7 @@ import { execFile as cpExecFile } from 'node:child_process' import fs from 'node:fs' import { promisify } from 'node:util' import { debug } from './debug' +import { createMemoizedPromiseContext } from './memoized-promise' const execFile = promisify(cpExecFile) const resolveStdout = ({ stdout }: { stdout: string }) => stdout.trim() @@ -27,67 +28,52 @@ export const getWindowsEnv = async (envName: string) => { type WSL2NetworkingMode = 'nat' | 'mirrored' -let wslNetworkingMode: - | WSL2NetworkingMode - | null - | Promise - | undefined +const wslNetworkingMode = + createMemoizedPromiseContext() -export const getWSL2NetworkingMode = async () => { - if (wslNetworkingMode === undefined) { - wslNetworkingMode = (async () => { - if ((await isWSL()) !== 2) return null - try { - return ( - await execFile('wslinfo', ['--networking-mode']).then(resolveStdout) - ).toLowerCase() as WSL2NetworkingMode - } catch (e) { - debug('Error while detecting WSL networking mode: %o', e) - return 'nat' // Default - } - })().then( - (correctedWSLNetworkingMode) => - (wslNetworkingMode = correctedWSLNetworkingMode) - ) - } - return await wslNetworkingMode -} +export const getWSL2NetworkingMode = () => + wslNetworkingMode.init(async () => { + if ((await isWSL()) !== 2) return null + + try { + return ( + await execFile('wslinfo', ['--networking-mode']).then(resolveStdout) + ).toLowerCase() as WSL2NetworkingMode + } catch (e) { + debug('Error while detecting WSL networking mode: %o', e) + return 'nat' // Default + } + }) -let isWsl: number | Promise | undefined +const isWsl = createMemoizedPromiseContext() const wsl2VerMatcher = /microsoft-standard-wsl2/i -export const isWSL = async (): Promise => { - if (isWsl === undefined) { - isWsl = (async () => { - if ((await import('is-wsl')).default) { - // Detect whether WSL version is 2 - // https://github.com/microsoft/WSL/issues/4555#issuecomment-700213318 - const isWSL2 = await (async () => { - if (process.env.WSL_DISTRO_NAME && process.env.WSL_INTEROP) - return true +export const isWSL = () => + isWsl.init(async () => { + if ((await import('is-wsl')).default) { + // Detect whether WSL version is 2 + // https://github.com/microsoft/WSL/issues/4555#issuecomment-700213318 + const isWSL2 = await (async () => { + if (process.env.WSL_DISTRO_NAME && process.env.WSL_INTEROP) return true - try { - const verStr = await fs.promises.readFile('/proc/version', 'utf8') - if (wsl2VerMatcher.test(verStr)) return true + try { + const verStr = await fs.promises.readFile('/proc/version', 'utf8') + if (wsl2VerMatcher.test(verStr)) return true - const gccMatched = verStr.match(/gcc[^,]+?(\d+)\.\d+\.\d+/) - if (gccMatched && Number.parseInt(gccMatched[1], 10) >= 8) - return true - } catch (e) { - debug('Error while detecting WSL version: %o', e) - debug('Assuming current WSL version is the primary version 2') - return true - } - })() + const gccMatched = verStr.match(/gcc[^,]+?(\d+)\.\d+\.\d+/) + if (gccMatched && Number.parseInt(gccMatched[1], 10) >= 8) return true + } catch (e) { + debug('Error while detecting WSL version: %o', e) + debug('Assuming current WSL version is the primary version 2') + return true + } + })() - const wslVersion = isWSL2 ? 2 : 1 - debug('Detected WSL version: %s', wslVersion) + const wslVersion = isWSL2 ? 2 : 1 + debug('Detected WSL version: %s', wslVersion) - return wslVersion - } else { - return 0 - } - })().then((correctedIsWsl) => (isWsl = correctedIsWsl)) - } - return await isWsl -} + return wslVersion + } else { + return 0 + } + }) From fddba47d161e0754c9ad83617d6f6ee8c976cccf Mon Sep 17 00:00:00 2001 From: Yuki Hattori Date: Tue, 14 Jan 2025 19:49:47 +0900 Subject: [PATCH 2/7] Allow configuration of parallelism in conversion Set the maximum number of parallel conversion for multiple files as `--parallel`/`-P` option. --- src/browser/manager.ts | 8 ++++---- src/config.ts | 18 ++++++++++++++++++ src/converter.ts | 17 ++++++++++++----- src/marp-cli.ts | 22 +++++++++++++++++++--- 4 files changed, 53 insertions(+), 12 deletions(-) diff --git a/src/browser/manager.ts b/src/browser/manager.ts index 1a00c11c..aa94d0de 100644 --- a/src/browser/manager.ts +++ b/src/browser/manager.ts @@ -111,14 +111,14 @@ export class BrowserManager implements AsyncDisposable { async dispose() { await Promise.all([ - async () => { + (async () => { await (await this._conversionBrowser.value)?.close() this._conversionBrowser.value = undefined - }, - async () => { + })(), + (async () => { await (await this._previewBrowser.value)?.close() this._previewBrowser.value = undefined - }, + })(), ]) } diff --git a/src/config.ts b/src/config.ts index 09acb3b5..641a1546 100644 --- a/src/config.ts +++ b/src/config.ts @@ -43,6 +43,7 @@ interface IMarpCLIArguments { notes?: boolean ogImage?: string output?: string | false + parallel?: number | boolean pdf?: boolean pdfNotes?: boolean pdfOutlines?: boolean @@ -94,6 +95,8 @@ export type IMarpCLIConfig = Overwrite< } > +export const DEFAULT_PARALLEL = 5 + export class MarpCLIConfig { args: IMarpCLIArguments = {} conf: IMarpCLIConfig = {} @@ -332,10 +335,25 @@ export class MarpCLIConfig { return scale })() + const parallel = (() => { + if (this.args.parallel === true) return DEFAULT_PARALLEL + if (this.args.parallel === false) return 1 + if (typeof this.args.parallel === 'number') + return Math.max(1, this.args.parallel) + + if (this.conf.parallel === true) return DEFAULT_PARALLEL + if (this.conf.parallel === false) return 1 + if (typeof this.conf.parallel === 'number') + return Math.max(1, this.conf.parallel) + + return 1 + })() + return { imageScale, inputDir, output, + parallel, pdfNotes, pdfOutlines, preview, diff --git a/src/converter.ts b/src/converter.ts index 1e857c80..8436692b 100644 --- a/src/converter.ts +++ b/src/converter.ts @@ -270,17 +270,24 @@ export class Converter { if (!inputDir && output && output !== '-' && files.length > 1) error('Output path cannot specify with processing multiple files.') - const parallel = Math.min(1, this.options.parallel ?? 1) + const parallel = Math.max(1, this.options.parallel ?? 1) const queue = [...files] - const workers = Array.from({ length: parallel }, async () => { - while (queue.length > 0) { - const file = queue.shift() - if (file) await this.convertFile(file, opts) + const workers = Array.from({ length: parallel }, async (_, i) => { + debug(`[Worker ${i + 1}] Start processing ...`) + + let file: File | undefined + + while ((file = queue.shift())) { + debug(`[Worker ${i + 1}] Processing ${file.absolutePath} ...`) + await this.convertFile(file, opts) } + + debug(`[Worker ${i + 1}] Finish processing.`) }) await Promise.all(workers) + debug(`Batch processing has been completed.`) } private convertFileToHTML(tpl: TemplateResult, file: File): File { diff --git a/src/marp-cli.ts b/src/marp-cli.ts index 61da1c92..edea3833 100644 --- a/src/marp-cli.ts +++ b/src/marp-cli.ts @@ -2,7 +2,7 @@ import chalk from 'chalk' import { availableFinders } from './browser/finder' import { BrowserManager } from './browser/manager' import * as cli from './cli' -import { fromArguments } from './config' +import { DEFAULT_PARALLEL, fromArguments } from './config' import { Converter, ConvertedCallback, ConvertType } from './converter' import { CLIError, CLIErrorCode, error, isError } from './error' import { File, FileType } from './file' @@ -172,6 +172,18 @@ export const marpCli = async ( describe: 'Prevent looking up for a configuration file', group: OptionGroup.Basic, }, + parallel: { + alias: ['P'], + default: DEFAULT_PARALLEL, + describe: 'Number of max parallel processes for multiple conversions', + group: OptionGroup.Basic, + type: 'number', + }, + 'no-parallel': { + describe: 'Disable parallel processing', + group: OptionGroup.Basic, + type: 'boolean', + }, watch: { alias: 'w', describe: 'Watch input markdowns for changes', @@ -472,7 +484,12 @@ export const marpCli = async ( if (cvtOpts.server) { await converter.convertFiles(foundFiles, { onlyScanning: true }) } else { - cli.info(`Converting ${length} markdown${length > 1 ? 's' : ''}...`) + const isParallel = Math.min(converter.options.parallel ?? 1, length) > 1 + + cli.info( + `Converting ${length} markdown${length > 1 ? 's' : ''}...${isParallel ? ` (Parallelism: up to ${converter.options.parallel} workers)` : ''}` + ) + await converter.convertFiles(foundFiles, { onConverted }) } } catch (e: unknown) { @@ -551,7 +568,6 @@ export const marpCli = async ( })().catch(rej) ) } - return 0 } catch (e: unknown) { if (throwErrorAlways || !(e instanceof CLIError)) throw e From 313e7efbea0542e7e91d304134595896a74bb3a9 Mon Sep 17 00:00:00 2001 From: Yuki Hattori Date: Wed, 15 Jan 2025 09:29:06 +0900 Subject: [PATCH 3/7] Test `Converter#convertFiles` with parallelism --- test/converter.ts | 84 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 73 insertions(+), 11 deletions(-) diff --git a/test/converter.ts b/test/converter.ts index e2cd3301..ef909cff 100644 --- a/test/converter.ts +++ b/test/converter.ts @@ -85,12 +85,13 @@ describe('Converter', () => { imageScale: 2, lang: 'fr', options: { html: true } as Options, + parallel: 3, server: false, template: 'test-template', templateOption: {}, type: ConvertType.html, watch: false, - } + } as const satisfies ConverterOption expect(new Converter(options).options).toMatchObject(options) }) @@ -1386,17 +1387,50 @@ describe('Converter', () => { }) describe('#convertFiles', () => { + const originalConvertFile = Converter.prototype.convertFile + const convertFileCallTimes: number[] = [] + + let convertFileSpy: jest.SpiedFunction + + beforeEach(() => { + convertFileCallTimes.splice(0, convertFileCallTimes.length) + + convertFileSpy = jest + .spyOn(Converter.prototype, 'convertFile') + .mockImplementation(function (this: Converter, ...args) { + convertFileCallTimes.push(Date.now()) + return originalConvertFile.apply(this, args) + }) + }) + describe('with multiple files', () => { - it('converts passed files', async () => { - await instance().convertFiles([new File(onePath), new File(twoPath)]) - expect(fs.promises.writeFile).toHaveBeenCalledTimes(2) - expect(writeFileSpy.mock.calls[0][0]).toBe( - `${onePath.slice(0, -3)}.html` - ) - expect(writeFileSpy.mock.calls[1][0]).toBe( - `${twoPath.slice(0, -6)}.html` - ) - }) + it( + 'converts passed files', + async () => { + await instance({ type: ConvertType.pdf }).convertFiles([ + new File(onePath), + new File(twoPath), + ]) + expect(fs.promises.writeFile).toHaveBeenCalledTimes(2) + + // Check sequential conversion + expect(convertFileSpy).toHaveBeenCalledTimes(2) + expect(convertFileCallTimes).toHaveLength(2) + expect( + Math.abs(convertFileCallTimes[1] - convertFileCallTimes[0]) + ).toBeGreaterThanOrEqual(300) + + const files = writeFileSpy.mock.calls.map(([fn]) => fn) + expect(files).toHaveLength(2) + expect(files).toStrictEqual( + expect.arrayContaining([ + `${onePath.slice(0, -3)}.pdf`, + `${twoPath.slice(0, -6)}.pdf`, + ]) + ) + }, + timeout + ) it('throws CLIError when output is defined', () => expect( @@ -1417,6 +1451,34 @@ describe('Converter', () => { expect(fs.promises.writeFile).not.toHaveBeenCalled() expect(stdout).toHaveBeenCalledTimes(2) }) + + describe('with parallel option', () => { + it( + 'converts passed files in parallel with the number of specified workers', + async () => { + await instance({ parallel: 2, type: ConvertType.pdf }).convertFiles( + [new File(onePath), new File(twoPath)] + ) + + // Check parallelism + expect(convertFileSpy).toHaveBeenCalledTimes(2) + expect(convertFileCallTimes).toHaveLength(2) + expect( + Math.abs(convertFileCallTimes[1] - convertFileCallTimes[0]) + ).toBeLessThan(300) + + const files = writeFileSpy.mock.calls.map(([fn]) => fn) + expect(files).toHaveLength(2) + expect(files).toStrictEqual( + expect.arrayContaining([ + `${onePath.slice(0, -3)}.pdf`, + `${twoPath.slice(0, -6)}.pdf`, + ]) + ) + }, + timeout + ) + }) }) }) }) From d2b6ca548be30457a48127ef287773d63c670202 Mon Sep 17 00:00:00 2001 From: Yuki Hattori Date: Wed, 15 Jan 2025 10:31:43 +0900 Subject: [PATCH 4/7] Test for `--parallel` and `-P` option --- src/config.ts | 20 +++++++++++--------- src/marp-cli.ts | 2 +- test/marp-cli.ts | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/src/config.ts b/src/config.ts index 641a1546..13978ab6 100644 --- a/src/config.ts +++ b/src/config.ts @@ -336,17 +336,19 @@ export class MarpCLIConfig { })() const parallel = (() => { - if (this.args.parallel === true) return DEFAULT_PARALLEL - if (this.args.parallel === false) return 1 - if (typeof this.args.parallel === 'number') - return Math.max(1, this.args.parallel) + const parseParallel = (value: boolean | number | undefined) => { + if (value === true) return DEFAULT_PARALLEL + if (value === false) return 1 + if (typeof value === 'number') return Math.max(1, value) - if (this.conf.parallel === true) return DEFAULT_PARALLEL - if (this.conf.parallel === false) return 1 - if (typeof this.conf.parallel === 'number') - return Math.max(1, this.conf.parallel) + return undefined + } - return 1 + return ( + parseParallel(this.args.parallel) ?? + parseParallel(this.conf.parallel) ?? + DEFAULT_PARALLEL + ) })() return { diff --git a/src/marp-cli.ts b/src/marp-cli.ts index edea3833..1ef4b1c4 100644 --- a/src/marp-cli.ts +++ b/src/marp-cli.ts @@ -174,7 +174,7 @@ export const marpCli = async ( }, parallel: { alias: ['P'], - default: DEFAULT_PARALLEL, + defaultDescription: DEFAULT_PARALLEL.toString(), describe: 'Number of max parallel processes for multiple conversions', group: OptionGroup.Basic, type: 'number', diff --git a/test/marp-cli.ts b/test/marp-cli.ts index 94f7ec97..cde7b4da 100644 --- a/test/marp-cli.ts +++ b/test/marp-cli.ts @@ -1141,6 +1141,42 @@ describe('Marp CLI', () => { }) }) + for (const opt of ['--parallel', '-P']) { + describe(`with ${opt} option`, () => { + it('converts files in parallel with specified concurrency', async () => { + expect((await conversion(opt, '2', onePath)).options.parallel).toBe(2) + }) + + it('converts files in parallel with 5 concurrency if set as true', async () => { + expect((await conversion(onePath, opt)).options.parallel).toBe(5) + }) + + it('converts files in serial if set as 1', async () => { + expect((await conversion(opt, '1', onePath)).options.parallel).toBe(1) + }) + + it('converts files in serial if set invalid value', async () => { + expect((await conversion(opt, '-1', onePath)).options.parallel).toBe( + 1 + ) + }) + }) + } + + describe('without parallel option', () => { + it('converts files in parallel with 5 concurrency', async () => { + expect((await conversion(onePath)).options.parallel).toBe(5) + }) + }) + + describe('with --no-parallel option', () => { + it('converts files in serial', async () => { + expect( + (await conversion('--no-parallel', onePath)).options.parallel + ).toBe(1) + }) + }) + describe('with -o option', () => { it('converts file and output to stdout when -o is "-"', async () => { const stdout = jest.spyOn(process.stdout, 'write').mockImplementation() From fe75475cd2b5af2df9a7d03a7e6e7ce0e0fb21b5 Mon Sep 17 00:00:00 2001 From: Yuki Hattori Date: Wed, 15 Jan 2025 20:48:39 +0900 Subject: [PATCH 5/7] Enable parallelism in CI tests --- .circleci/config.yml | 2 +- .github/workflows/test-win.yml | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 52244e08..6798fbc6 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -87,7 +87,7 @@ commands: steps: - run: name: Jest - command: npm run test:coverage -- --ci -i --reporters=default --reporters=jest-junit + command: npm run test:coverage -- --ci --reporters=default --reporters=jest-junit environment: JEST_JUNIT_OUTPUT_DIR: tmp/test-results MARP_TEST_CI: 1 diff --git a/.github/workflows/test-win.yml b/.github/workflows/test-win.yml index b0b53ec9..dce89f2f 100644 --- a/.github/workflows/test-win.yml +++ b/.github/workflows/test-win.yml @@ -43,7 +43,8 @@ jobs: env: MARP_TEST_CI: 1 run: >- - npm run test:coverage -- --ci -i --reporters=default --reporters=jest-junit --forceExit || + npm run test:coverage -- --ci --reporters=default --reporters=jest-junit || + npm run test:coverage -- --ci -i --reporters=default --reporters=jest-junit --forceExit --no-cache || npm run test:coverage -- --ci -i --reporters=default --reporters=jest-junit --forceExit --no-cache || npm run test:coverage -- --ci -i --reporters=default --reporters=jest-junit --forceExit --no-cache From 56a55a5eca5463bb039d1b9279282540082320df Mon Sep 17 00:00:00 2001 From: Yuki Hattori Date: Wed, 15 Jan 2025 21:15:46 +0900 Subject: [PATCH 6/7] Update CI test settings --- .circleci/config.yml | 12 +++++++++--- .github/workflows/test-win.yml | 6 +++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 6798fbc6..5448e178 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -84,10 +84,14 @@ commands: command: npm run lint:css test: + parameters: + runInBand: + type: boolean + default: false steps: - run: name: Jest - command: npm run test:coverage -- --ci --reporters=default --reporters=jest-junit + command: npm run test:coverage -- --ci --reporters=default --reporters=jest-junit <<# parameters.runInBand >>-i<> environment: JEST_JUNIT_OUTPUT_DIR: tmp/test-results MARP_TEST_CI: 1 @@ -129,7 +133,8 @@ jobs: - prepare: browser: true - lint - - test + - test: + runInBand: true test-node20: executor: @@ -140,7 +145,8 @@ jobs: - prepare: browser: true - lint - - test + - test: + runInBand: true test-node22: executor: diff --git a/.github/workflows/test-win.yml b/.github/workflows/test-win.yml index dce89f2f..68475e57 100644 --- a/.github/workflows/test-win.yml +++ b/.github/workflows/test-win.yml @@ -24,6 +24,10 @@ jobs: # - name: Output concurrency group # run: echo "${{ github.workflow }}-${{ (github.ref_name == 'main' && github.run_id) || format('{0}-{1}', github.actor, github.head_ref || github.ref_name) }}" + - name: Get number of CPU cores + uses: SimenB/github-actions-cpu-cores@v2 + id: cpu-cores + - uses: actions/checkout@v4 - name: Use Node.js ${{ matrix.node-version }} @@ -43,7 +47,7 @@ jobs: env: MARP_TEST_CI: 1 run: >- - npm run test:coverage -- --ci --reporters=default --reporters=jest-junit || + npm run test:coverage -- --ci --max-workers ${{ steps.cpu-cores.outputs.count }} --reporters=default --reporters=jest-junit || npm run test:coverage -- --ci -i --reporters=default --reporters=jest-junit --forceExit --no-cache || npm run test:coverage -- --ci -i --reporters=default --reporters=jest-junit --forceExit --no-cache || npm run test:coverage -- --ci -i --reporters=default --reporters=jest-junit --forceExit --no-cache From 62d4d4d5fa105432955220ca1f4ca2fe9361de24 Mon Sep 17 00:00:00 2001 From: Yuki Hattori Date: Wed, 15 Jan 2025 21:30:49 +0900 Subject: [PATCH 7/7] [ci skip] Update README.md and CHANGELOG.md --- CHANGELOG.md | 4 ++++ README.md | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4718a002..2e9b37a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Introduce parallelism for batch conversion: `--parallel` / `-P` ([#509](https://github.com/marp-team/marp-cli/issues/509), [#628](https://github.com/marp-team/marp-cli/pull/628)) + ### Fixed - Make the preview option stable against occasional invalid URL errors ([#627](https://github.com/marp-team/marp-cli/pull/627)) diff --git a/README.md b/README.md index b865de4c..2bb3b9af 100644 --- a/README.md +++ b/README.md @@ -256,6 +256,10 @@ marp --pdf --allow-local-files slide-deck.md ## Conversion modes +### Parallelism + +When converting multiple files, Marp CLI will process them in parallel with 5 concurrency by default. You can set the number of concurrency by `--parallel` (`-P`) option, or disable parallelism by `--no-parallel`. + ### Watch mode (`--watch` / `-w`) Marp CLI will observe a change of Markdown and using theme CSS when passed with `--watch` (`-w`) option. The conversion will be triggered whenever the content of file is updated.