freshOS / Stevia

:leaves: Concise Autolayout code
https://freshos.github.io/SteviaDocs/
MIT License
3.38k stars 214 forks source link

remove deprecation warnings on all double-dash operators #165

Closed cowgp closed 2 years ago

cowgp commented 2 years ago

I use Stevia in a rather large app and have recently been digging into why my compile times are terribly slow. For context, doing a "clean build" takes ~9 minutes on my laptop. I've been going through with the swift compiler flag for -Xfrontend -debug-time-function-bodies to help assist where our issues lie and it turns out that all my (beloved) Stevia layout blocks are a huge part of my problem.

Way back in the swift 2 timeframe, we had to have the -- operator because XCode would fail to compile saying "the statement was to complex to evaluate" (assume you remember that well @s4cha ). While XCode has long since stopped choking like that, it turns out that the -- is still massively superior to the - when you look at compile times.

For example, I have a method that has 8 lines of Stevia's chaining APIs followed by this simple layout block with 3 views on one line. This method is taking 4,057ms to compile! (yes 4 seconds).

private func configureViewLayout() {
    contentView.fillVertically().fillHorizontally(padding: 12)
    contentStackView.fillContainer()
    petImageStackView.fillContainer(padding: 4)
    flashlightButtonContentView.size(50)
    petImageView.centerInContainer().size(25)
    lightIndicator.centerInContainer().size(32)
    disclosureIndicator.width(10)
    petPillView.fillHorizontally().centerInContainer()

    petInfoContainerView.layout {
        0
        |-0 - petStatusImageView.width(14) - 8 - petNameLabel - 8 - batteryIndicator.height(14) - 0-|
        0
    }
}

If I change the main line of the layout block to use the double dash -- operator like the following, then compile time on this method drops to 37ms. (yes it drops from 4075 to 37, that's huge, shaving 99% off the compile time)

petInfoContainerView.layout {
    0
    |-0 -- petStatusImageView.width(14.0) -- 8 -- petNameLabel -- 8 -- batteryIndicator.height(14.0) -- 0-|
    0
}

I've got north of 150 views that I'm scrubbing through and switching to double-dashes and it's going to end up saving multiple minutes of compile time - which is outstanding...

...but every instance of -- is now generating compiler warning - so I've suddenly got hundreds of compiler warnings about the -- being deprecated.

Given this evidence, I think it's really relevant to un-deprecate the double-dash operator as it definitely has an important use.

hope you agree.

cowgp commented 2 years ago

As a tangental side note that is potentially worth including in the Stevia documentation is that number literals - Integers vs doubles - makes a significant difference in compile time - but the opposite of what I expected.

Technically all (most?) numerical values in Stevia eventually end up as CGFloats cause that's what autolayout uses - so I would have assumed that using doubles would be friendlier to the compiler... something like 8.0 would seem easier to infer and more logical, but it turns out that using doubles in a layout block is significantly slower than integers.

I'm wondering if this is because there are operator overloads in the dash and doubledash for both Double and CGFloat and the compiler has to spend time figuring out which one to use - but an 8 instead of an 8.0 is easier on the compiler because there's only one option to infer, it's an Int.

So for context, the example above that I used had 0 and 8 as padding values in the layout block. but if I switch those to 0.0 and 8.0, then my compile time goes up from 37ms to 114ms... 3x compile time!

So lesson there is padding values in layout blocks should always be integers if you care about compile times. (this is assuming you are using number literals. if you declare them as variables first outside the layout block, and thus the value is typed, there is no hit)

Interestingly - with the chaining APIs though, it makes no impact wether you use doubles or ints. So just the padding values in layout blocks

cowgp commented 2 years ago

I am now understanding the comment here: https://github.com/freshOS/Stevia/blob/master/Sources/Stevia/Stevia%2BDoubleDash.swift#L142

You added a "Hyphen bullet" and are ultimately trying to drive people to a "Hyphen bullet" instead of the double dash.

However I disagree with this decision and stand by the request of this PR to remove the deprecation on the double dash because my keyboard does not readily have any ability to type a "hyphen bullet", nor is it visually different enough to know when reviewing code whether somebody used a minus or a hyphen-bullet. Relying on the double dash correctly enables both of those things.

s4cha commented 2 years ago

Hey @cowgp, thank you so much for your thorough feedback on this.

You are on point with your analysis. The more variants available, the more time the compiler takes to find the correct one, leading to crazy compile times. The regular "-" operator "collides" with the native one which certainly has many overloads already. That's why the "chaining" apis have no cost compared to the type-safe visual apis.

I initially deprecated the double dash operator to avoid having too many ways to do the same thing but your feedback does make sense I totally agree that the "hyphen bullet", while visually pleasing, is not user friendly and actually misleading. Let's bring the double dash back !

s4cha commented 2 years ago

Thanks for issuing the PR on top of that, this is really appreciated !

s4cha commented 2 years ago

This is now live in release 5.1.2