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

PR test in mdn/translated-content is slower than before #8301

Closed
3 tasks done
yin1999 opened this issue Feb 24, 2023 · 6 comments
Closed
3 tasks done

PR test in mdn/translated-content is slower than before #8301

yin1999 opened this issue Feb 24, 2023 · 6 comments
Labels
dx developer experience p1 We will address this soon and will provide capacity from our team for it in the next few releases. perf performance issues regression wx writer experience

Comments

@yin1999
Copy link
Member

yin1999 commented Feb 24, 2023

Summary

I just notice that after we migrating to ESM. The PR test workflow in translated-content is slower that before.

See the following workflow run:

As @queengooborg mentioned in translated-content, maybe we need synchronous file reading for this step.

URL

None

Reproduction steps

See workflow run.

Expected behavior

The workflow could run faster (maybe we could start from the gather translation step).

Actual behavior

The workflow is a bit slow.

Device

Desktop

Browser

Chrome

Browser version

Stable

Operating system

Linux

Screenshot

No response

Anything else?

No response

Validations

queengooborg added a commit to queengooborg/yari that referenced this issue Feb 24, 2023
This changes the `findAll()` function to an asynchronous function to help improve performance.

Partial fix to mdn#8301.
@caugner
Copy link
Contributor

caugner commented Feb 24, 2023

Please note that individual workflow runs are not necessarily comparable, as workflow runners use a variety of CPUs.

@yin1999
Copy link
Member Author

yin1999 commented Feb 24, 2023

Please note that individual workflow runs are not necessarily comparable, as workflow runners use a variety of CPUs.

Sorry about ignoring that, I just made a local test, and found that:

run node node_modules/@mdn/yari/build/cli.js files/zh-tw/learn/common_questions/web_mechanics/what_is_a_web_server/index.md

use @mdn/[email protected]:

13.639s is used to gather translations.

image

use @mdn/[email protected]:

1.831s is used to gather translations.

image

Test platform:

OS: Linux debian 6.1.0-3-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.8-1 (2023-01-29) x86_64 GNU/Linux
Node.js: v18.13.0
Yarn: 1.22.19 (Debian package: [email protected]+~cs24.27.18-2)
CPU: 13th Gen Intel(R) Core(TM) i5-13600K

With 1TB SSD and 32GB memory

@caugner caugner added p1 We will address this soon and will provide capacity from our team for it in the next few releases. dx developer experience wx writer experience perf performance issues regression and removed needs-triage labels Feb 24, 2023
@caugner
Copy link
Contributor

caugner commented Feb 24, 2023

I compared @mdn/[email protected] and @mdn/[email protected] and indeed the build performance has degraded significantly:

  • 1.36.4: 19.0 seconds
  • 2.0.0: 49.0 seconds

@queengooborg
Copy link
Collaborator

queengooborg commented Feb 25, 2023

On bf23810, I ran yarn build files/zh-tw/learn/common_questions/web_mechanics/what_is_a_web_server/index.md and the command completed in ~19 seconds:

❯ yarn build files/zh-tw/learn/common_questions/web_mechanics/what_is_a_web_server/index.md
yarn run v1.22.19
$ cross-env NODE_ENV=production ts-node build/cli.ts files/zh-tw/learn/common_questions/web_mechanics/what_is_a_web_server/index.md
(node:36300) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
CONTENT_ROOT:            /Users/queengooborg/Developer/Gooborg/mdn-content/files
CONTENT_TRANSLATED_ROOT: /Users/queengooborg/Developer/Gooborg/mdn-translated-content/files
Time to gather all translations: 16.302s
Language does not exist: svelte
/Users/queengooborg/Developer/Gooborg/yari/client/build/zh-tw/docs/learn/common_questions/web_mechanics/what_is_a_web_server
Built 1 pages in 17.5 seconds, at a rate of 0.1 documents per second.
Peak heap memory usage: 300 MB


Total_Flaws_Count
broken_links  8
macros        1


✨  Done in 19.45s.

The time spent gathering all the translations is definitely quite high, I wonder what's causing the sudden change in build times, hmm...

@queengooborg
Copy link
Collaborator

queengooborg commented Feb 25, 2023

In v2.5.5 (the latest as of this comment) on a brand new package and folder made specifically for testing Yari, I found that the time spent went down significantly to ~4.3s:

❯ node node_modules/@mdn/yari/build/cli.js files/zh-tw/learn/common_questions/web_mechanics/what_is_a_web_server/index.md
(node:46939) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
CONTENT_ROOT:            /Users/queengooborg/Developer/Gooborg/mdn-content/files
CONTENT_TRANSLATED_ROOT: /Users/queengooborg/Developer/Gooborg/mdn-translated-content/files
Time to gather all translations: 4.300s
Language does not exist: svelte
/Users/queengooborg/Developer/tmp-yari-test/node_modules/@mdn/yari/client/build/zh-tw/docs/learn/common_questions/web_mechanics/what_is_a_web_server
Built 1 pages in 5.2 seconds, at a rate of 0.2 documents per second.
Peak heap memory usage: 252 MB


Total_Flaws_Count
broken_links  8
macros        1

And in v1.36.4, I get around the same time.


When I was trying to dig into profiling and performance, I ran a profiler through Chrome's debug panel (within Yari, using the latest branch), and I found something interesting:

Screenshot 2023-02-25 at 05 26 40

I don't see much of a reason that the front-matter package would be causing such an issue... Maybe it's something to do with NodeJS wrapping CJS to ESM?

@yin1999
Copy link
Member Author

yin1999 commented May 19, 2023

After the fix (#8806) is applied, the build time is reduced to 3.5s. Let's see if there is side-effect made by the fix for a few days. If nothing happend, I'll close this PR :)

image

@yin1999 yin1999 closed this as completed May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dx developer experience p1 We will address this soon and will provide capacity from our team for it in the next few releases. perf performance issues regression wx writer experience
Projects
None yet
Development

No branches or pull requests

3 participants