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.

Sign up for GitHub

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

Jump to bottom

disable incremental build for legacy tsconfig.json #82986

Merged
merged 2 commits into from
Nov 18, 2020
Merged

disable incremental build for legacy tsconfig.json #82986

merged 2 commits into from
Nov 18, 2020

Conversation

mshustov
Copy link
Contributor

@mshustov mshustov commented Nov 9, 2020

Summary

@mattkime reported that incremental build for the kibana project takes a long time. In theory, the size of the project might be a problem: the project includes almost all the OSS plugins. As a result, the tsbuildinfo cache file size is 5.9Mb.
I'd suggest disabling incremental mode for tsconfig.json, but keep it for projects under src/*. tsconfig.json won't include any files in the future, but list all the TS projects as references.

@mshustov mshustov added chore Team:Operations Team label for Operations Team v8.0.0 7.11.0 labels Nov 9, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@mshustov mshustov added the release_note:skip Skip the PR/issue when compiling release notes label Nov 9, 2020
@jen-huang jen-huang added v7.11.0 and removed 7.11.0 labels Nov 10, 2020
@mshustov mshustov requested a review from a team November 11, 2020 13:40
@spalger
Copy link
Contributor

spalger commented Nov 11, 2020 •
edited
Loading

I wanted to understand the impact this will have on bootstrap time so I picked a kind of arbitrary transition on master, 34c80e5 -> af51394 and measured the bootstrap time (without yarn install) after merging in af51394 with and without this commit:

  • without this commit: 16 sec
  • with this commit: 51 sec

This is kinda troubling, and seems to be contrary to the goal of this PR...

@spalger
Copy link
Contributor

spalger commented Nov 12, 2020

@restrry I can't find a way to disable incremental builds only for the type_check script (no way to unset the tsBuildInfoFile from what I can tell using CLI flags), but we should be able to pass the compiler options necessary to enable incremental builds only for bootstrap, which I'm guessing would keep the performance of incremental in bootstrap and improve performance of the complete scripts/type_check.

@mshustov
Copy link
Contributor Author

mshustov commented Nov 16, 2020 •
edited
Loading

but we should be able to pass the compiler options necessary to enable incremental builds only for bootstrap

According to the docs https://www.typescriptlang.org/docs/handbook/compiler-options.html incremental builds are disabled if composite: false which we do set in type_check script

kibana/src/dev/typescript/run_type_check_cli.ts

Line 87 in ae8f8e1

...['--composite', 'false'],

But incremental builds work apparently. Anyway, I'm not sure we want to disabler incremental builds for other projects with composite: true that would benefit from them.

no way to unset the tsBuildInfoFile from what I can tell using CLI flags

We can remove tsBuildInfoFile from tsconfig.json file what this PR does 🤷‍♂️ This project is compiled during type_check only.

spalger
spalger approved these changes Nov 18, 2020
View reviewed changes
Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

Chatted with @restrry and it seems my measurements were wrong and this doesn't impact bootstrap negatively. There is some caching happening that I wasn't aware of and it seems that is what lead to the shorter bootstrap time without this change.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@mshustov mshustov merged commit 938b762 into elastic:master Nov 18, 2020
@mshustov mshustov deleted the disable-incremental-ts-for-legacy-config branch November 18, 2020 19:52
@mshustov mshustov mentioned this pull request Nov 18, 2020
Merged
mshustov added a commit to mshustov/kibana that referenced this pull request Nov 18, 2020
@mshustov
disable incremenetal build for legacy tsconfig.json (elastic#82986)
68a8c02
mshustov added a commit that referenced this pull request Nov 19, 2020
@mshustov
disable incremenetal build for legacy tsconfig.json (#82986) (#83711)
616d582
phillipb added a commit to phillipb/kibana that referenced this pull request Nov 19, 2020
@phillipb
Merge branch 'add-logs-to-node-details' of github.com:phillipb/kibana…
2249140 
… into add-logs-to-node-details

* 'add-logs-to-node-details' of github.com:phillipb/kibana: (87 commits)
  [Maps] Add 'crossed' & 'exited' events to tracking alert (elastic#82463)
  Updating code-owners to use new core/app-services team names (elastic#83731)
  Add Managed label to data streams and a view switch for the table (elastic#83049)
  [Maps] Add query bar inputs to geo threshold alerts tracked points & boundaries (elastic#80871)
  fix(NA): search examples kibana version declaration (elastic#83182)
  Fixed console error, which appears when saving changes in Edit Alert flyout (elastic#83610)
  [Alerting] Add `alert.updatedAt` field to represent date of last user edit (elastic#83578)
  Not resetting server log level if level is defined (elastic#83651)
  disable incremenetal build for legacy tsconfig.json (elastic#82986)
  [Workplace Search] Migrate SourceLogic from ent-search (elastic#83593)
  [Workplace Search] Port Box changes from ent-search (elastic#83675)
  [APM] Improve router types (elastic#83620)
  Bump flat to v4.1.1 (elastic#83647)
  Bump y18n@5 to v5.0.5 (elastic#83644)
  Bump jsonpointer to v4.1.0 (elastic#83641)
  Bump is-my-json-valid to v2.20.5 (elastic#83642)
  [Telemetry] Move Monitoring collection strategy to a collector (elastic#82638)
  Update typescript eslint to v4.8 (elastic#83520)
  [ML] Persist URL state for Anomaly detection jobs using metric function (elastic#83507)
  [ML] Performance improvements to annotations editing in Single Metric Viewer & buttons placement (elastic#83216)
  ...
chrisronline pushed a commit to chrisronline/kibana that referenced this pull request Nov 19, 2020
@mshustov @chrisronline
disable incremenetal build for legacy tsconfig.json (elastic#82986)
0614bfa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spalger spalger spalger approved these changes

Assignees
No one assigned
Labels
chore release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v7.11.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mshustov @elasticmachine @spalger @kibanamachine @jen-huang