Skip to content

Revert recursive protocol constraints changes #10950

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 14, 2017
Merged

Revert recursive protocol constraints changes #10950

merged 2 commits into from
Jul 14, 2017

Conversation

rudkx
Copy link
Contributor

@rudkx rudkx commented Jul 13, 2017

The type checking time for Swift.o is more than three times longer than it was before these changes. Backing out until Doug has a chance to take a look.

rudkx added 2 commits July 13, 2017 16:05
@rudkx
Revert "SE-0157: Enable recursive protocol constraints by default."
ed23eb3 
This reverts commit afbdbae.

Commit ded45a6 more than triples the
type checking time when building Swift.o, so I am going to revert that ,
and it looks like this needs to be reverted as well if that commit is
reverted.
@rudkx
Revert "[GSB] SE-0157: Reprocess delayed requirements when we need a …
5acc140 
…complete PA."

This reverts commit ded45a6.

This seems to slow down type checking of Swift.o by more than 3x.
@rudkx
Copy link
Contributor Author

rudkx commented Jul 13, 2017

@swift-ci Please smoke test and merge

This was referenced Jul 13, 2017
Merged
Merged
@rudkx
Copy link
Contributor Author

rudkx commented Jul 14, 2017

@shahmishal Mishal, any idea why this would have picked up changes from @harlanhaskins PR? He tells me these changes aren't in master yet. https://ci.swift.org/job/swift-PR-osx-smoke-test/9474/

@rudkx
Copy link
Contributor Author

rudkx commented Jul 14, 2017

@swift-ci Please smoke test OS X platform

@rudkx rudkx merged commit b37b3ba into swiftlang:master Jul 14, 2017
@rudkx rudkx deleted the revert-recursive-protocol-constraints-changes branch July 14, 2017 06:21
@johnno1962
Copy link
Contributor

Not sure this was the problem. I’ve building https://github.com/SwiftJava using a toolchain from master over the last few days. Xcode9b3 5mins build time. Before this revert 9 mins. After this revert 11 mins.

@rudkx
Copy link
Contributor Author

rudkx commented Jul 14, 2017

I verified before backing this out that ded45a6 was the cause of the 3x slow-down in building the stdlib.

I don't know how much generic code SwiftJava has, but the stdlib uses generics extensively and it would not be surprising to me that changes to the handling of generics that substantially regress the build time for the stdlib would not also regress the build time for other projects.

@johnno1962
Copy link
Contributor

johnno1962 commented Jul 14, 2017 •
edited
Loading

I may be wrong, this code doesn’t use generics that much. I’m just noticing that a toolchain built with utils/build-toolchain on master seems a lot slower than the last beta. Would that be expected?

@rudkx
Copy link
Contributor Author

rudkx commented Jul 14, 2017

Yes. Toolchains are built with assertions and more expensive verification code enabled.

@johnno1962
Copy link
Contributor

Ah, ok. Sorry for the noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rudkx @johnno1962