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

[POC] Introduce Task Workers #961

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
121 changes: 55 additions & 66 deletions lib/processors/minifier.js
Original file line number Diff line number Diff line change
@@ -1,63 +1,44 @@
import {fileURLToPath} from "node:url";

Check failure on line 1 in lib/processors/minifier.js

View workflow job for this annotation

GitHub Actions / General checks, tests and coverage reporting

'fileURLToPath' is defined but never used
import posixPath from "node:path/posix";
import {promisify} from "node:util";
import os from "node:os";
import workerpool from "workerpool";
import Resource from "@ui5/fs/Resource";
import {getLogger} from "@ui5/logger";
const log = getLogger("builder:processors:minifier");
import {setTimeout as setTimeoutPromise} from "node:timers/promises";

Check failure on line 5 in lib/processors/minifier.js

View workflow job for this annotation

GitHub Actions / General checks, tests and coverage reporting

'setTimeoutPromise' is defined but never used
import {minify} from "terser";

/**
* @private
* @module @ui5/builder/tasks/minifyWorker
*/

/**
* Preserve comments which contain:
* <ul>
* <li>copyright notice</li>
* <li>license terms</li>
* <li>"@ui5-bundle"</li>
* <li>"@ui5-bundle-raw-include"</li>
* </ul>
*
* @type {RegExp}
*/
const copyrightCommentsAndBundleCommentPattern = /copyright|\(c\)(?:[0-9]+|\s+[0-9A-Za-z])|released under|license|\u00a9|^@ui5-bundle-raw-include |^@ui5-bundle /i;

const debugFileRegex = /((?:\.view|\.fragment|\.controller|\.designtime|\.support)?\.js)$/;

const MIN_WORKERS = 2;
const MAX_WORKERS = 4;
const osCpus = os.cpus().length || 1;
const maxWorkers = Math.max(Math.min(osCpus - 1, MAX_WORKERS), MIN_WORKERS);

Check failure on line 31 in lib/processors/minifier.js

View workflow job for this annotation

GitHub Actions / General checks, tests and coverage reporting

'maxWorkers' is assigned a value but never used

const sourceMappingUrlPattern = /\/\/# sourceMappingURL=(\S+)\s*$/;
const httpPattern = /^https?:\/\//i;

// Shared workerpool across all executions until the taskUtil cleanup is triggered
let pool;

Check failure on line 37 in lib/processors/minifier.js

View workflow job for this annotation

GitHub Actions / General checks, tests and coverage reporting

'pool' is defined but never used

function getPool(taskUtil) {
if (!pool) {
log.verbose(`Creating workerpool with up to ${maxWorkers} workers (available CPU cores: ${osCpus})`);
const workerPath = fileURLToPath(new URL("./minifierWorker.js", import.meta.url));
pool = workerpool.pool(workerPath, {
workerType: "auto",
maxWorkers
});
taskUtil.registerCleanupTask((force) => {
const attemptPoolTermination = async () => {
log.verbose(`Attempt to terminate the workerpool...`);

if (!pool) {
return;
}

// There are many stats that could be used, but these ones seem the most
// convenient. When all the (available) workers are idle, then it's safe to terminate.
let {idleWorkers, totalWorkers} = pool.stats();
while (idleWorkers !== totalWorkers && !force) {
await setTimeoutPromise(100); // Wait a bit workers to finish and try again
({idleWorkers, totalWorkers} = pool.stats());
}

const poolToBeTerminated = pool;
pool = null;
return poolToBeTerminated.terminate(force);
};

return attemptPoolTermination();
});
}
return pool;
}

async function minifyInWorker(options, taskUtil) {

Check failure on line 40 in lib/processors/minifier.js

View workflow job for this annotation

GitHub Actions / General checks, tests and coverage reporting

'minifyInWorker' is defined but never used
return getPool(taskUtil).exec("execMinification", [options]);

Check failure on line 41 in lib/processors/minifier.js

View workflow job for this annotation

GitHub Actions / General checks, tests and coverage reporting

'getPool' is not defined
}

async function extractAndRemoveSourceMappingUrl(resource) {
Expand All @@ -66,8 +47,8 @@
const sourceMappingUrlMatch = resourceContent.match(sourceMappingUrlPattern);
if (sourceMappingUrlMatch) {
const sourceMappingUrl = sourceMappingUrlMatch[1];
if (log.isLevelEnabled("silly")) {

Check failure on line 50 in lib/processors/minifier.js

View workflow job for this annotation

GitHub Actions / General checks, tests and coverage reporting

'log' is not defined
log.silly(`Found source map reference in content of resource ${resourcePath}: ${sourceMappingUrl}`);

Check failure on line 51 in lib/processors/minifier.js

View workflow job for this annotation

GitHub Actions / General checks, tests and coverage reporting

'log' is not defined
}

// Strip sourceMappingURL from the resource to be minified
Expand All @@ -93,13 +74,13 @@
// (which it is but inlined)
return Buffer.from(base64Content, "base64").toString();
} else {
log.warn(

Check failure on line 77 in lib/processors/minifier.js

View workflow job for this annotation

GitHub Actions / General checks, tests and coverage reporting

'log' is not defined
`Source map reference in resource ${resourcePath} is a data URI but has an unexpected` +
`encoding: ${sourceMappingUrl}. Expected it to start with ` +
`"data:application/json;charset=utf-8;base64,"`);
}
} else if (httpPattern.test(sourceMappingUrl)) {
log.warn(`Source map reference in resource ${resourcePath} is an absolute URL. ` +

Check failure on line 83 in lib/processors/minifier.js

View workflow job for this annotation

GitHub Actions / General checks, tests and coverage reporting

'log' is not defined
`Currently, only relative URLs are supported.`);
} else if (posixPath.isAbsolute(sourceMappingUrl)) {
log.warn(`Source map reference in resource ${resourcePath} is an absolute path. ` +
Expand Down Expand Up @@ -159,24 +140,12 @@
* Promise resolving with object of resource, dbgResource and sourceMap
*/
export default async function({
resources, fs, taskUtil, options: {readSourceMappingUrl = false, addSourceMappingUrl = true, useWorkers = false
} = {}}) {
let minify;
resources, fs, options: {readSourceMappingUrl = false, addSourceMappingUrl = true, useWorkers = false
} = {}, log, resourceFactory}) {
if (readSourceMappingUrl && !fs) {
throw new Error(`Option 'readSourceMappingUrl' requires parameter 'fs' to be provided`);
}

if (useWorkers) {
if (!taskUtil) {
// TaskUtil is required for worker support
throw new Error(`Minifier: Option 'useWorkers' requires a taskUtil instance to be provided`);
}
minify = minifyInWorker;
} else {
// Do not use workerpool
minify = (await import("./minifierWorker.js")).default;
}

return Promise.all(resources.map(async (resource) => {
const resourcePath = resource.getPath();
const dbgPath = resourcePath.replace(debugFileRegex, "-dbg$1");
Expand Down Expand Up @@ -248,7 +217,7 @@
sourceMapJson.file = dbgFilename;

// Then create a new resource
dbgSourceMapResource = new Resource({
dbgSourceMapResource = resourceFactory.createResource({
string: JSON.stringify(sourceMapJson),
path: dbgPath + ".map"
});
Expand All @@ -265,19 +234,39 @@
}
}
}

const result = await minify({
filename,
dbgFilename,
code,
sourceMapOptions
}, taskUtil);
resource.setString(result.code);
const sourceMapResource = new Resource({
path: resource.getPath() + ".map",
string: result.map
});
return {resource, dbgResource, sourceMapResource, dbgSourceMapResource};
try {
const result = await minify({
// Use debug-name since this will be referenced in the source map "sources"
[dbgFilename]: code
}, {
output: {
comments: copyrightCommentsAndBundleCommentPattern,
wrap_func_args: false
},
compress: false,
mangle: {
reserved: [
"jQuery",
"jquery",
"sap",
]
},
sourceMap: sourceMapOptions
});
resource.setString(result.code);
const sourceMapResource = resourceFactory.createResource({
path: resource.getPath() + ".map",
string: result.map
});
return {resource, dbgResource, sourceMapResource, dbgSourceMapResource};
} catch (err) {
// Note: err.filename contains the debug-name
throw new Error(
`Minification failed with error: ${err.message} in file ${filename} ` +
`(line ${err.line}, col ${err.col}, pos ${err.pos})`, {
cause: err
});
}
}));
}

Expand Down
13 changes: 5 additions & 8 deletions lib/tasks/minify.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
import minifier from "../processors/minifier.js";
import fsInterface from "@ui5/fs/fsInterface";

/**
* @public
* @module @ui5/builder/tasks/minify
Expand All @@ -16,6 +13,7 @@ import fsInterface from "@ui5/fs/fsInterface";
* @param {object} parameters Parameters
* @param {@ui5/fs/DuplexCollection} parameters.workspace DuplexCollection to read and write files
* @param {@ui5/project/build/helpers/TaskUtil|object} [parameters.taskUtil] TaskUtil
* @param {object} parameters.processors
* @param {object} parameters.options Options
* @param {string} parameters.options.pattern Pattern to locate the files to be processed
* @param {boolean} [parameters.options.omitSourceMapResources=false] Whether source map resources shall
Expand All @@ -26,13 +24,12 @@ import fsInterface from "@ui5/fs/fsInterface";
* @returns {Promise<undefined>} Promise resolving with <code>undefined</code> once data has been written
*/
export default async function({
workspace, taskUtil, options: {pattern, omitSourceMapResources = false, useInputSourceMaps = true
}}) {
workspace, taskUtil, processors, options: {pattern, omitSourceMapResources = false, useInputSourceMaps = true}
}) {
const resources = await workspace.byGlob(pattern);
const processedResources = await minifier({
const processedResources = await processors.execute("minifier", {
resources,
fs: fsInterface(workspace),
taskUtil,
reader: workspace,
options: {
addSourceMappingUrl: !omitSourceMapResources,
readSourceMappingUrl: !!useInputSourceMaps,
Expand Down
24 changes: 22 additions & 2 deletions lib/tasks/taskRepository.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {createRequire} from "node:module";
import {fileURLToPath} from "node:url";

/**
* Repository providing access to all UI5 Builder tasks and various metadata required by the build process.
Expand All @@ -21,7 +22,14 @@ const taskInfos = {
executeJsdocSdkTransformation: {path: "./jsdoc/executeJsdocSdkTransformation.js"},
generateApiIndex: {path: "./jsdoc/generateApiIndex.js"},
generateJsdoc: {path: "./jsdoc/generateJsdoc.js"},
minify: {path: "./minify.js"},
minify: {
path: "./minify.js",
processors: {
minifier: {
path: "../processors/minifier.js"
}
}
},
buildThemes: {path: "./buildThemes.js"},
transformBootstrapHtml: {path: "./transformBootstrapHtml.js"},
generateLibraryManifest: {path: "./generateLibraryManifest.js"},
Expand Down Expand Up @@ -67,8 +75,20 @@ export async function getTask(taskName) {
}
try {
const {default: task} = await import(taskInfo.path);
let processors = null;
if (taskInfo.processors) {
processors = Object.create(null);
for (const processorName in taskInfo.processors) {
if (Object.hasOwn(taskInfo.processors, processorName)) {
processors[processorName] = {
path: fileURLToPath(new URL(taskInfo.processors[processorName].path, import.meta.url))
};
}
}
}
return {
task
task,
processors
};
} catch (err) {
throw new Error(`taskRepository: Failed to require task module for ${taskName}: ${err.message}`);
Expand Down
Loading