Skip to content

Improve build performance #83

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

Merged
merged 6 commits into from
Nov 8, 2024
Merged

Improve build performance #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c97691c
perf: avoid large hole array in bidiMirroringGlyphMap
JLHwung Nov 6, 2024
89544f1
perf: use native JSON.stringify for gzipped data
JLHwung Nov 6, 2024
1f4e8ab
minor cleanup
JLHwung Nov 7, 2024
c6f9130
build: exit with non-zero code if worker throws
JLHwung Nov 7, 2024
f7fdf25
test: throw an error
JLHwung Nov 7, 2024
89937b0
Revert "test: throw an error"
JLHwung Nov 7, 2024
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
23 changes: 20 additions & 3 deletions scripts/generate-data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

const resources = require('../data/resources.js');
const generateData = require('../index.js');
const utils = require('../scripts/utils.js');

// -----------------------------------------------------------------------------

Expand All @@ -28,7 +27,9 @@ const complicatedWorkThatTakesTime = (resource, callback) => {
console.log('[%s] Worker %d \u2192 Unicode v%s',
getTime(), cluster.worker.id, version);

console.groupCollapsed();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As a bonus, this refactor also fixes another build hazard: Previously when there is an error thrown from the building worker, the cluster main does not exit early, nor does it exit with non-zero code. Now that all building steps are within one process, any thrown errors should invalidate the build step.

Copy link
Contributor Author

@JLHwung JLHwung Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I have to revert the remove cluster commit because I overlooked the total time reported by the zsh time. There is still improvement from multiple processes. I will see if I can fix the build hazard directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed the mentioned build hazard in c6f9130.

See https://github.com/node-unicode/node-unicode-data/actions/runs/11724848834/job/32659747285 for the error log example.

generateData(version);
console.groupEnd();

complicatedWorkThatTakesTime(
resource.slice(1),
Expand All @@ -40,10 +41,16 @@ const complicatedWorkThatTakesTime = (resource, callback) => {
}
};

if (cluster.isMaster) {
if (cluster.isPrimary) {

for (let index = 0; index < numCPUs; index++) {
cluster.fork();
const worker = cluster.fork();
worker.on('message', (error) => {
for (const id in cluster.workers) {
cluster.workers[id].kill();
}
throw new Error(`Worker ${worker.id} encountered an error: ${error}`);
})
}

cluster.on('online', (worker) => {
Expand Down Expand Up @@ -76,4 +83,14 @@ if (cluster.isMaster) {
});
});

process.on('uncaughtException', (error) => {
console.error(error);
process.send(error.message);
});

process.on('unhandledRejection', (error) => {
console.error(error);
process.send(error.message || error);
});

}
71 changes: 38 additions & 33 deletions scripts/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const gzipInline = function(data) {
if (data instanceof Map) {
return `new Map(${ gzipInline([...data]) })`;
}
const json = jsesc(data, { 'json': true });
const json = JSON.stringify(data);
const gzipBuffer = zlib.gzipSync(json);
const str = gzipBuffer.toString('base64');
return `JSON.parse(require('zlib').gunzipSync(Buffer.from('${ str }','base64')))`;
Expand Down Expand Up @@ -84,7 +84,13 @@ const writeFiles = function(options) {
return;
}
const dirMap = {};
const bidiMirroringGlyphMap = [];
/**
* A list of flatten (x, y) pairs,
* where x is a codepoint, y := codepoint(z) - x,
* where z is the BidiMirroringGlyph of character(x) and codepoint(z) > x
* @type number[]
*/
const bidiMirroringGlyphFlatPairs = [];
const auxMap = {};
Object.keys(map).forEach(function(item) {
const codePoints = map[item];
Expand Down Expand Up @@ -112,10 +118,11 @@ const writeFiles = function(options) {
)
) {
if (type == 'Bidi_Mirroring_Glyph') {
const shortName = item.codePointAt(0);
const toCodepoint = item.codePointAt(0);
codePoints.toArray().forEach(function(codePoint) {
console.assert(!bidiMirroringGlyphMap[codePoint]);
bidiMirroringGlyphMap[codePoint] = shortName;
if (codePoint < toCodepoint) {
bidiMirroringGlyphFlatPairs.push(codePoint, toCodepoint - codePoint);
}
});
} else {
if (!auxMap[type]) {
Expand All @@ -124,34 +131,7 @@ const writeFiles = function(options) {
auxMap[type].push([item, codePoints]);
}
}
if (isNamesCanon) {
return;
}

if (type == 'Bidi_Mirroring_Glyph') {
const dir = path.resolve(
__dirname, '..',
'output', 'unicode-' + version, type
);
if (!hasKey(dirMap, type)) {
dirMap[type] = [];
}
fs.mkdirSync(dir, { recursive: true });
// `Bidi_Mirroring_Glyph/index.js`
// Note: `Bidi_Mirroring_Glyph` doesn’t have repeated strings; don’t gzip.
const flatPairs = bidiMirroringGlyphMap
.flatMap((a, b) => a < b ? [a, b - a] : []);
const output = [
`const chr=String.fromCodePoint`,
`const pair=(t,u,v)=>[t?u+v:v,chr(t?u:u+v)]`,
`module.exports=new Map(${
jsesc(flatPairs)
}.map((v,i,a)=>pair(i&1,a[i^1],v)))`
].join(';');
fs.writeFileSync(
path.resolve(dir, 'index.js'),
output
);
if (isNamesCanon || type == 'Bidi_Mirroring_Glyph') {
return;
}

Expand Down Expand Up @@ -211,6 +191,31 @@ const writeFiles = function(options) {
`module.exports=${ symbolsExports }`
);
});
if (options.type == 'Bidi_Mirroring_Glyph') {
const type = options.type;
const dir = path.resolve(
__dirname, '..',
'output', 'unicode-' + version, type
);
if (!hasKey(dirMap, type)) {
dirMap[type] = [];
}
fs.mkdirSync(dir, { recursive: true });
// `Bidi_Mirroring_Glyph/index.js`
// Note: `Bidi_Mirroring_Glyph` doesn’t have repeated strings; don’t gzip.
const output = [
`const chr=String.fromCodePoint`,
`const pair=(t,u,v)=>[t?u+v:v,chr(t?u:u+v)]`,
`module.exports=new Map(${
JSON.stringify(bidiMirroringGlyphFlatPairs)
}.map((v,i,a)=>pair(i&1,a[i^1],v)))`
].join(';');
fs.writeFileSync(
path.resolve(dir, 'index.js'),
output
);
return;
}
Object.keys(auxMap).forEach(function(type) {
const dir = path.resolve(
__dirname, '..',
Expand Down