Improve build performance - #83
Conversation
| for (const resource of resources) { | ||
| const version = resource.version; | ||
| console.log('[%s] Unicode v%s', getTime(), version); | ||
| console.groupCollapsed(); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
mathiasbynens
left a comment
mathiasbynens
left a comment
There was a problem hiding this comment.
LGTM to merge after rebase. Thanks a lot for your continued work on this!
In node-unicode#80 we made a performance regression that the bidiMirroringGlyphMap is generated in the property iteration. This is also fixed.
c6c50a9 to
89544f1
Compare
|
Rebuilding all packages after merging this, I noticed that |
No, this should be a bug. I will open a new PR. |
2 participants
This PR contains commits from #82. I will rebase once that PR gets merged.
This PR fixed a performance regression introduced in #80 where we generated the bidi_mirroring_glyph data when enumerating the glyph mappings, although the final data is correct, this is apparently not desired.
We also removed the cluster building approach because at this point, it is more efficient to run the V8 optimized code again and again for several seconds than forking multiple V8 processes simutaneously optimizing code which does not run for a very long time.Last we used the native
JSON.stringifywhen generating the gzipped data. Because the data is gzipped and not human-readable anyway, there is not very much benefit runningjsescbefore gzip.Unsurprisingly, the last commit generates lots of output diffs. However, they are equivalent for end users, so I didn't attach the output diff in this PR.
Runtime measured in my local environment (Apple M1 Max 10c)
The time spent on each task when processing Unicode 16.0.0 in this PR (Click to unfold)