feat(cli): support multithreading in all cli commands#2320
Conversation
…to extract-to-template
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2320 +/- ##
==========================================
- Coverage 77.05% 76.20% -0.85%
==========================================
Files 84 99 +15
Lines 2157 2639 +482
Branches 555 685 +130
==========================================
+ Hits 1662 2011 +349
- Misses 382 504 +122
- Partials 113 124 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I fixed the obvious issue, in my local case time dropped from minutes (it was ten minutes when i stopped it, so it's probably even longer) to 17 seconds. @Zxilly please test it on your codebase. I also did few cpu profiles, and have interesting results: Locale consisting from 300 catalogs, with 2k messages was compiled in approximately 6 seconds on my machine.
|
|
@Zxilly did you have a chance to test the lates changes on your project? What the results? @andrii-bodnar what is needed to make this PR to be merged? |
|
Sorry I forgot to sync progress. The project's |
|
@timofei-iatsenko I'm planning a new release by the end of this week. I will merge this PR soon if everything is ready. |
|
@Zxilly you don't need to extract profile from the CI, if you have access to the project locally, just run it localy like so And than import cpu profile file here https://discoveryjs.github.io/cpupro/ |
node --prof ./node_modules/@lingui/cli/dist/lingui compile --workers 4 |
node --max-old-space-size=16384 --prof ./node_modules/@lingui/cli/dist/lingui extract --workers 4 |
|
The profile should looks diffrently, it should be |
|
Yes, I am collecting CPU profile data, but company policy requires me to mask certain portions of the data, so I need to perform additional processing. |
|
Based on my observations, approximately 40% of the time spent in the In the worker, nearly all time is spent idle. Perhaps this is because esbuild is actively working, and Node.js cannot track its internal calls? |
| function hexToBase64(hexStr: string) { | ||
| let base64 = "" | ||
| const base64: string[] = [] | ||
|
|
||
| for (let i = 0; i < hexStr.length; i++) { | ||
| base64 += !((i - 1) & 1) | ||
| ? String.fromCharCode(parseInt(hexStr.substring(i - 1, i + 1), 16)) | ||
| : "" | ||
| base64.push( | ||
| !((i - 1) & 1) | ||
| ? String.fromCharCode(parseInt(hexStr.substring(i - 1, i + 1), 16)) | ||
| : "" | ||
| ) | ||
| } | ||
| return btoa(base64) | ||
|
|
||
| return btoa(base64.join("")) | ||
| } |
There was a problem hiding this comment.
Maybe we can just
Buffer.from(hexStr, 'hex').toString('base64');There was a problem hiding this comment.
Oh, I noticed the explanation at #1776. Maybe we could try dynamically require Buffer when it's available?
There was a problem hiding this comment.
Basically even if this is conditional, bundlers should bundle the code behind this require. And because it's native node module - it will fail. Some additional configuration of bundler would be required to overcome this issue and that something i want to avoid.
I also noticed that, but not sure is it something i can improve. I mean in my perf test scenario i'm indeed have a lot of PO files, and parsing them should take some significant time. Hard to say time taken now is reasonable or it could be improved.
Have no idea, but sounds realistic. |
|
I created a more efficient export function hexToBase64(hexStr: string) {
let binaryString = "";
for (let i = 0; i < hexStr.length; i += 2) {
const hexPair = hexStr.substring(i, i + 2);
binaryString += String.fromCharCode(parseInt(hexPair, 16));
}
return btoa(binaryString);
}Benchmark result: Using |
|
Meanwhile, |
|
If using the polyfill from Perhaps we could use this polyfill for long strings while falling back to the simple implementation for short strings? |
|
The following code reports such performance. import { fromHex, toBase64 } from "hextreme";
export function hexToBase64(hexStr: string) {
if (hexStr.length >= 256) {
return toBase64(fromHex(hexStr));
}
let binaryString = "";
for (let i = 0; i < hexStr.length; i += 2) {
const hexPair = hexStr.substring(i, i + 2);
binaryString += String.fromCharCode(parseInt(hexPair, 16));
}
return btoa(binaryString);
} |
|
@Zxilly have you tested how much impact it has in the real world scenario? Ho much speedup it gives if you use this faster implementation? I think i could add node-only version using package json export conditions or simply using a subentry for node. But as far as i remember it doesn't take a significant amount of time in the whole program execution. |
|
@timofei-iatsenko @Zxilly do we expect any changes due to the ongoing discussion, or is everything ready? |
|
I believe it's ready. This ongoing discussion won't deliver significant overall performance improvements, but perhaps we can submit these detailed optimizations at a later stage. |
|
Thank you both for your hard work on this, @Zxilly and @timofei-iatsenko! |
|
I just wanted to say a huge THANK YOU for everything that you all have been doing over the last few weeks for these performance improvements! |
Description
Since i rewrite almost everethying from the original PR, i'm reopening this from myself.
Feature description
This PR adds support of thread pool (using threads-js) for compile, extract, extract-template and experimental extractor commands.
Here is how tasks split between workers in commands:
compile: by locale. So en, ru, pl, etc will go each into it's own worker. Why not by files (catalogs)? Because of the mergeCatalogs feature (where all files merged into one) and because, as i believe the message compiling itself doesn't take too much time, but serializing this into the file can do (BTW, this is for the future iterations, that shouldn't take that long). In any way, splitting by locales should give very even distribution across workers because amount of job for every locale is the same.extract/extract-template: by source filesexperimental-extractor: by bundles, each bundle after esbuild are processed by separate worker in the pool. So if experimental extractor used for pages in the MPA applications, every page first bundled by esbuild and then processed in parallel by worker poolMulthreading is enabled by default. Users can disable or configure it's behavior using
--workersargument for cli. Docs were added.Other notable changes:
Due to the fact that you can send between workers / main thread only serializable data structures, you could not send a class with methods. So i did some refactorings to internal structures to use less classes/instances and more plain structures and pure functions.
Types of changes
Fixes # (issue)
Checklist