robb / Cartography

A declarative Auto Layout DSL for Swift :iphone::triangular_ruler:
Other
7.35k stars 524 forks source link

Adds custom operators for speed #293

Open acrookston opened 6 years ago

acrookston commented 6 years ago

Addresses #215.

This is maybe not the best solution (not DRY) but with the code below we've seen swift type checking times go down from 8800ms+ to 39ms (example at bottom).

With this PR I wanted to start a discussion around using custom operators. As I understand it, the problem is the overloading of common operators like ==, - and + causing confusion for the type checker and massive compile times.

Other than tests, not sure what is missing but I'd be happy to make any improvements.

Example code

A fairly complex example, here seen with the updated syntax.

constrain([self, titleLabel, dialCodeButton, phoneTextField, submitButton]) { proxies in
            var views = proxies
            let superview = views.removeFirst()
            let titleLabel = views.removeFirst()
            let dialCodeButton = views.removeFirst()
            let phoneTextField = views.removeFirst()
            let submitButton = views.removeFirst()

            for view in [titleLabel, submitButton] {
                view.left ~== superview.left ~+ GUI.Constant.tableInset
                view.right ~== superview.right ~- GUI.Constant.tableInset
            }

            dialCodeButton.left ~== superview.left ~+ GUI.Constant.tableInset
            phoneTextField.left ~== dialCodeButton.right ~+ padding
            phoneTextField.right ~== superview.right ~- GUI.Constant.tableInset

            dialCodeButton.width ~== (superview.width ~- (GUI.Constant.tableInset * 2)) ~* 0.2
            phoneTextField.width ~== (superview.width ~- (GUI.Constant.tableInset * 2)) ~* 0.8

            submitButton.height ~== GUI.Constant.buttonHeight
            phoneTextField.height ~== GUI.Constant.buttonHeight
            dialCodeButton.height ~== GUI.Constant.buttonHeight

            titleLabel.top ~== (superview.top + GUI.Constant.tableInset)
            phoneTextField.top ~== (titleLabel.bottom + GUI.Constant.tableInset)
            dialCodeButton.top ~== (titleLabel.bottom + GUI.Constant.tableInset)
            submitButton.top ~== (phoneTextField.bottom + padding)
        }
acrookston commented 6 years ago

Another example, much simpler takes 3600ms.

constrain(self, textLabel, helpButton) { superview, textLabel, helpButton in
            let textWidth = superview.width - iconWidth - padding - GUI.Constant.tableInset * 2
            textLabel.left == superview.left + GUI.Constant.tableInset
            helpButton.width == iconWidth
            textLabel.width == textWidth
            helpButton.right == superview.right - GUI.Constant.tableInset
            textLabel.centerY == superview.centerY
            helpButton.centerY == superview.centerY
            textLabel.height == GUI.Constant.rowHeight
            helpButton.height == GUI.Constant.rowHeight
        }
acrookston commented 6 years ago

After migrating most of the code in one of our apps over to the new syntax the build time has been halved (from about 5 min to 2.5). All of our view constraints all run in a few milliseconds where most were in the hundreds, if not thousands. Strongly recommend the core team looks over this issue and applies some (maybe not my) solution. Thanks for a great library!

LucasVanDongen commented 6 years ago

Always felt it was due to something like this. It's a pity. But the == operator is a lie anyway as with a true == it doesn't matter what you put left or right and that clearly isn't the case with Cartography.

orta commented 6 years ago

Is it this a semver break?

acrookston commented 6 years ago

@orta technically no, but it's adding to the API and suggesting a new API so "perhaps"? With the current iteration I'm not replacing the current operators, I've only copied or "aliased" them (hence non-DRY). Was hoping to spark a discussion on how this would be best implemented upstream. I'm willing to take lead on any required changes.

orta commented 6 years ago

I'm basically the person running this repo, but I've never used it, nor really grok it's trade-offs as I've been absent from Swift for enough years.

So I'm happy to go with consensus, and those compilation speed improvements make any pain worth it IMO - it's bad enough without a library making it worse. Did tests pass for you locally? Do we need to update travis?

fssilva commented 5 years ago

Is there an ETA for this fix to be merge ?

orta commented 5 years ago

No ETA, maybe a rebase might fix CI though