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

Simplify expression in STPSectionHeaderView.swift #1846

Merged
merged 1 commit into from
Jul 21, 2021
Merged

Simplify expression in STPSectionHeaderView.swift #1846

merged 1 commit into from
Jul 21, 2021

Conversation

JonathanDowning
Copy link
Contributor

@JonathanDowning JonathanDowning commented Jul 20, 2021

Summary

I've simplified an expression in this view to reduce the compile time from 13 seconds to 6 milliseconds.

Motivation

I analyze the compile time of my projects from time to time and discovered Stripe currently has a hotspot.

After making this innocuous change, the time to compile goes from:
12899.38ms stripe-ios/Stripe/STPSectionHeaderView.swift:124:10 instance method height(forButtonText:width:)
to:
6.05ms stripe-ios/Stripe/STPSectionHeaderView.swift:123:18 instance method height(forButtonText:width:)

Testing

The code is identical in functionality.

@davidme-stripe
Copy link
Contributor

Thanks for catching this and cleaning it up! We'll publish it and credit you in the next update.

This code was automatically converted from Objective-C using a tool, and the results were a little odd in some places. We'll run through with -debug-time-function-bodies and try to fix a few of the other hotspots.

@davidme-stripe davidme-stripe added the triaged Issue has been reviewed by Stripe and is being tracked internally label Jul 20, 2021
@JonathanDowning
Copy link
Contributor Author

It's a pleasure @davidme-stripe! Thanks for all the hard work y'all do.

@mludowise-stripe mludowise-stripe merged commit f68ae6c into stripe:master Jul 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triaged Issue has been reviewed by Stripe and is being tracked internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants