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

[BUG] osd/optimizer takes too long(~5 min) to respond #2223

Closed
abbyhu2000 opened this issue Aug 30, 2022 · 10 comments · Fixed by #2319
Closed

[BUG] osd/optimizer takes too long(~5 min) to respond #2223

abbyhu2000 opened this issue Aug 30, 2022 · 10 comments · Fixed by #2319
Assignees
Labels
bug Something isn't working high priority v2.4.0 'Issues and PRs related to version v2.4.0'

Comments

@abbyhu2000
Copy link
Member

Describe the bug

When developing in the wizard folder, the osd/optimizer package has inconsistent behaviors. It takes about 5 min to compile the bundles after yarn start; or it will take 5 min to respond to any changes. However, once it compiles, it will work as normal( can detect changes and restart immiediately)

To Reproduce
Steps to reproduce the behavior:

  1. Start the application by "yarn opensearch snapshot" and "yarn start"
  2. The log will show "http server running at http://localhost..." but "1 bundles compiled successfully after 329.2 sec, watching for changes..." shown as below

Screen Shot 2022-08-30 at 10 07 37 AM

Or:

  1. Make some changes in the public wizard folder, for example, add a console.log("Reload")
  2. After saving the changed file, it takes around 5 min for the application to detect the changes, restart and show the console.log message
    image

Expected behavior
The osd/optimizer should respond faster and constantly watches for changes.

Additional context
The problem only occurs when I am developing in the wizard plugin folder.

I have tried a couple ways to debug:

  1. Set my file watcher limit to the maximum.
  2. Add a line for wizard in the limits.yml in the osd-optimizer folder Screen Shot 2022-08-30 at 10 34 25 AM
@kristenTian
Copy link
Contributor

[Triage]: let's follow check if the optimizer is the problem. #2224

Check if tsconfig is different and if it's doing sth different during compelling

@ashwin-pc
Copy link
Member

Some additional context. It looks like its not just a wizard plugin problem but a app problem in general. The first build for a plugin takes much longer than subsequent builds for it. And the time taken depends on the plugin. e.g. the agg_params_helper takes 30s while wizard takes 330s and visualize_editor_common takes 90s.

# src/plugins/vis_default_editor/public/components/agg_params_helper.ts
np bld    log   [xx:xx:xx.xxx] [success][@osd/optimizer] 1 bundles compiled successfully after 30.2 sec, watching for changes
np bld    log   [xx:xx:xx.xxx] [success][@osd/optimizer] 1 bundles compiled successfully after 0.1 sec, watching for changes

# src/plugins/wizard/public/application/components/data_tab/secondary_panel.tsx
np bld    log   [xx:xx:xx.xxx] [success][@osd/optimizer] 1 bundles compiled successfully after 327.4 sec, watching for changes

# deleted tsconfig in src/plugins/wizard/ (No change)
np bld    log   [xx:xx:xx.xxx] [success][@osd/optimizer] 1 bundles compiled successfully after 330.6 sec, watching for changes

# src/plugins/visualize/public/application/components/visualize_editor_common.tsx
np bld    log   [xx:xx:xx.xxx] [success][@osd/optimizer] 1 bundles compiled successfully after 91.5 sec, watching for changes

It is also the first build for that specific plugin that takes time and not just the first build after startup. so after startup is a plugin is built and then a change is made to anpther plugin, the new plugins change will take as long as it needs for a first build.

Subsequent changes are quite fast.

@ashwin-pc
Copy link
Member

Steps to repro consistently:

  • Stop the app
  • yarn start
  • Make change in plugin and save
  • Go to the app url: http://localhost:5601
  • Wait and see console output for bundling time

@abbyhu2000
Copy link
Member Author

abbyhu2000 commented Sep 9, 2022

After investigation, the compiling issue is not related to wizard plugin. However, it relates to this commit #2054. On the main branch, the compiling time takes around 20s before this commit; it takes around 6-7 min to compile or to detect any changes after this commit. Seem like using dart-sass instead of node-sass will slow compilation time significantly.

@abbyhu2000 abbyhu2000 changed the title [BUG] osd/optimizer takes too long(~5 min) to respond when developing in the wizard plugin folder [BUG] osd/optimizer takes too long(~5 min) to respond Sep 9, 2022
@joshuarrrr
Copy link
Member

nice find! Now we just have to figure out how to fix 😁

@joshuarrrr
Copy link
Member

joshuarrrr commented Sep 9, 2022

It looks like it may actually be specific to some of the SASS functions in EUI/OUI: sass/dart-sass#113 (comment)

We may see if we can remove some of the math functions in https://github.com/opensearch-project/oui/blob/321ee0736d3501134c4a4620bd10a78fc68c0a52/src/global_styling/functions/_math.scss (I'm not sure how widely used they are in the library)

@abbyhu2000
Copy link
Member Author

It looks like it may actually be specific to some of the SASS functions in EUI/OUI: sass/dart-sass#113 (comment)

We may see if we can remove some of the math functions in https://github.com/opensearch-project/oui/blob/321ee0736d3501134c4a4620bd10a78fc68c0a52/src/global_styling/functions/_math.scss (I'm not sure how widely used they are in the library)

Thank you! I will take a look

@ashwin-pc
Copy link
Member

This is interesting! Can we validate this? A few things that i've observed could be tied to this.

  1. Making a SCSS change takes ~30s
  2. Compiling the wizard plugin the first time takes ~300s
  3. Compiling other plugins like visualize take between ~3s to ~90s.

The dart sass theory could explain this. If the issue is indeed related to sass, can we remove the sass imports in any one plugin (e.g. wizard) and verify if the time to first compile is indeed faster.

@abbyhu2000
Copy link
Member Author

I validated that the slow compilation time issue indeed relates to sass. I removed all sass related imports in the wizard plugin(here is the PR in my own repo if anyone wishes to validate again: abbyhu2000#17) and compared the compilation time.

  1. Wizard without sass: It took 3.5s for the first compilation
  2. Wizard with sass: It took 409s for the first compilation

One possible solution is to add a node-fibers package. Pulling from the sass official website, it talks about "To avoid this performance hit, render() can use the fibers package to call asynchronous importers from the synchronous code path". And i have validated the performance improvement:

  1. Wizard with sass after adding node-fibers package: took 76s for the first compilation.

However, node-fiber package has reached end of life with Node 16. If the performance issue is a problem, we can consider stay on Node 14 and use node-fiber, and remove fibers when migrating to Node 16. Here is the PR: #2319

@abbyhu2000
Copy link
Member Author

Also tried replacing sass with sass-embedded, the first time compilation takes around 300s, which is not a big improvement from the original.

kavilla pushed a commit that referenced this issue Sep 13, 2022
For issue [#2223](#2223), in order to solve the slow compilation time that is caused by changing from node-sass to dart-sass, one possible solution is to add a node-fibers package. Pulling from the sass official website, "to avoid this performance hit, render() can use the fibers package to call asynchronous importers from the synchronous code path". And i have validated the performance improvement: the first compilation changed to 76s from the previous 409s.

However, node-fiber package has reached end of life with Node 16. If the performance issue is a problem, we can consider stay on Node 14 and use node-fiber for now, and remove fibers when migrating to Node 16 later.

Issue resolved:
#2223

Signed-off-by: abbyhu2000 <[email protected]>
sipopo pushed a commit to sipopo/OpenSearch-Dashboards that referenced this issue Sep 14, 2022
For issue [opensearch-project#2223](opensearch-project#2223), in order to solve the slow compilation time that is caused by changing from node-sass to dart-sass, one possible solution is to add a node-fibers package. Pulling from the sass official website, "to avoid this performance hit, render() can use the fibers package to call asynchronous importers from the synchronous code path". And i have validated the performance improvement: the first compilation changed to 76s from the previous 409s.

However, node-fiber package has reached end of life with Node 16. If the performance issue is a problem, we can consider stay on Node 14 and use node-fiber for now, and remove fibers when migrating to Node 16 later.

Issue resolved:
opensearch-project#2223

Signed-off-by: abbyhu2000 <[email protected]>
Signed-off-by: Sergey V. Osipov <[email protected]>
sipopo pushed a commit to sipopo/OpenSearch-Dashboards that referenced this issue Dec 16, 2022
For issue [opensearch-project#2223](opensearch-project#2223), in order to solve the slow compilation time that is caused by changing from node-sass to dart-sass, one possible solution is to add a node-fibers package. Pulling from the sass official website, "to avoid this performance hit, render() can use the fibers package to call asynchronous importers from the synchronous code path". And i have validated the performance improvement: the first compilation changed to 76s from the previous 409s.

However, node-fiber package has reached end of life with Node 16. If the performance issue is a problem, we can consider stay on Node 14 and use node-fiber for now, and remove fibers when migrating to Node 16 later.

Issue resolved:
opensearch-project#2223

Signed-off-by: abbyhu2000 <[email protected]>
Signed-off-by: Sergey V. Osipov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high priority v2.4.0 'Issues and PRs related to version v2.4.0'
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants