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

Shorten type checking times by shortening expressions and avoiding type conversions #5105

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

idrougge
Copy link

I noticed that DGCharts amounted to a large portion of the compile times of our app, so I measured the times for evaluating expressions and function bodies by adding the below build flags:

-Xfrontend -warn-long-function-bodies=<ms>
-Xfrontend -warn-long-expression-type-checking=<ms>

It turned out that certain functions and expressions had evaluation times close to one second on an M1 Mac, so I added some help for the compiler to shorten those checking times.

As a result of these changes, the compile times have gone down from 6 s for the entire package to only 4 s — a 33 % decrease. Certain files still make out the bulk of the total compile time so a further refactoring of those files could enable higher parallelisation of compilation, but that would make this PR too big to review.

Goals ⚽

Shorter compile times.

Implementation Details 🚧

  1. Bracketing parts of certain long expressions such as -0.5 * (sqrt(1.0 - position * position) - 1.0)
  2. Removing type conversions from Double to Double such as Double( sqrt(1 - position * position) )
  3. Adding a guard to functions that consisted of just one big if statement
  4. Adding a type specifier to variable declarations that instead used a type initialiser on a literal such as var maxW: CGFloat = 0.0

Testing Details 🔍

The tests still run as before.
To ease code review, it helps to turn on "hide white spaces" in Github's code review view.

@drewster99
Copy link

Oof. Similar work done in #5124.

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.

2 participants