Skip to content

Conversation

@avivkeller
Copy link
Member

Description

This PR offloads generators to worker threads. I've done a check of each generator, and nothing crashes, however, please please do a run through to verify nothing changed.

Because some functions cannot be cloned, toString() (on SemVer) has been replaced with .version, and similar for other SemVer related activities.

Validation

The output should be identical to the current generation.

Related Issues

See https://openjs-foundation.slack.com/archives/CVAMEJ4UV/p1744161814523689

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I've covered new added functionality with unit tests if necessary.

@avivkeller avivkeller requested a review from a team as a code owner April 10, 2025 15:40
Copy link
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

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

Mostly LGTM! But could you add benchmarks (simple time would suffice with different amount of threads) to check how much perf we gaining here.

@avivkeller
Copy link
Member Author

For the following command:

node ./bin/cli.mjs -i ../node/doc/api/*.md -t legacy-json -t legacy-html -o out

Where "Benchmark 1" ran with worker support, and "Benchmark 2" ran without:

Benchmark 1:
  Time (abs ≡):        47.460 s               [User: 95.622 s, System: 2.748 s]

Benchmark 2:
  Time (abs ≡):        68.457 s               [User: 77.903 s, System: 1.420 s]

Benchmark 1 is 1.44 times faster than than Benchmark 2

@avivkeller avivkeller merged commit 91df749 into main Apr 12, 2025
7 checks passed
@avivkeller avivkeller deleted the worker_threads branch April 12, 2025 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants