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

Huge performance impact due to whiteoutLeadingComments calls #3091

Closed
1 of 5 tasks
ansgarm opened this issue Oct 25, 2021 · 4 comments
Closed
1 of 5 tasks

Huge performance impact due to whiteoutLeadingComments calls #3091

ansgarm opened this issue Oct 25, 2021 · 4 comments
Assignees
Labels
bug This issue is a bug. closing-soon This issue will automatically close in 4 days unless further comments are made. needs-reproduction This issue needs reproduction. p1 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@ansgarm
Copy link
Contributor

ansgarm commented Oct 25, 2021

🐛 Bug Report

Affected Languages

  • TypeScript or Javascript
  • Python
  • Java
  • .NET (C#, F#, ...)
  • Go

(possibly others as well)

General Information

  • JSII Version: 1.37.0 (build a6fe12f), typescript 3.9.10
  • Platform: OS X

What is the problem?

Hi! I just did some CPU profiling of our cdktf get command as its execution time increased substantially over the course of the last months. The cause for this was probably the growing AWS Terraform provider – which can take up to an hour to build in CI nowadays.

It appears that calls to whiteoutLeadingComments take a lot of time:

image

I was able to confirm this locally by commenting out the body of whiteoutLeadingComments which brought the invocation of JSII (with Python configured) down to 90 seconds (from 1270s).

Is there a way we could e.g. introduce caching for the result of whiteoutLeadingComments or computeLineStarts to improve the performance?

Reproduce

To reproduce the long build times, clone repo and run cdktf get.

Verbose Log

@ansgarm ansgarm added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 25, 2021
@NGL321 NGL321 added the p1 label Oct 26, 2021
@indrora indrora self-assigned this May 19, 2022
@indrora indrora added needs-reproduction This issue needs reproduction. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels May 19, 2022
@indrora
Copy link

indrora commented May 19, 2022

This function appears to have either been renamed or rolled in elsewhere. Is this still a bottleneck?

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label May 20, 2022
@indrora indrora added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Oct 18, 2022
@github-actions
Copy link
Contributor

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Oct 19, 2022
@ansgarm
Copy link
Contributor Author

ansgarm commented Oct 19, 2022

I'd need to check – but I think we can close this one. All our issues that linked to this are closed by now, too. So if we revisit (i.e. profile) JSII performance at some point in the future, we'll just raise a new up-to-date issue if we find something like this again 👍

@ansgarm ansgarm closed this as completed Oct 19, 2022
@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. closing-soon This issue will automatically close in 4 days unless further comments are made. needs-reproduction This issue needs reproduction. p1 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

4 participants