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

Use interpolation instead of '+' concatenation for problematic expression #4123

Merged
merged 1 commit into from
Mar 18, 2020

Conversation

Jumhyn
Copy link
Contributor

@Jumhyn Jumhyn commented Aug 26, 2019

Goals ⚽

Reduces the type-checking time of a debugDescription variable by at least an order of magnitude.

Implementation Details 🚧

Because type inference with + and literals has to do a lot of overload checking (for every ExpressibleBy____Literal), many expressions which use + for string concatenation with literals in contexts where type information is sparse may inadvertently cause a type checker performance explosion. In this case, we are concatenating an anonymous closure argument with a string literal, which caused a compilation time on the order of multiple seconds (on my MBP, YMMV). With this change, the type checking of this expression completes in under 100ms.

Testing Details 🔍

There's no behavior change here. We could consider adding -Xfrontend -warn-long-expression-type-checking=<limit> to the Xcode project to catch warnings about type checker performance bottlenecks.

@codecov-io
Copy link

Codecov Report

Merging #4123 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #4123   +/-   ##
======================================
  Coverage    41.5%   41.5%           
======================================
  Files         119     119           
  Lines       13992   13992           
======================================
  Hits         5807    5807           
  Misses       8185    8185
Impacted Files Coverage Δ
...Charts/Data/Implementations/ChartBaseDataSet.swift 25.82% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd1bb78...f0adde5. Read the comment docs.

@liuxuan30
Copy link
Member

liuxuan30 commented Nov 5, 2019

@jjatie any feedback? Since it's just for debugging info, there is no performance change in reality.

@StephanVV
Copy link

Running xcodebuild with the following flags shows it takes a while to compile

OTHER_SWIFT_FLAGS="-Xfrontend -debug-time-function-bodies"

2346.70ms
Pods/Charts/Source/Charts/Data/Implementations/ChartBaseDataSet.swift:404:5 getter debugDescription

2180.13ms
Pods/Charts/Source/Charts/Data/Implementations/ChartBaseDataSet.swift:404:5 getter debugDescription

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.

5 participants