-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Enable serialization of witness tables by default #12493
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
Enable serialization of witness tables by default #12493
Conversation
Now that we remove the [serialized] flag from functions after their early serialization, we can run another round of optimizations on them. Due to this change, it should be OK to serialize witness tables now because marking the witness methods [serialized] does not affect how well they can be optimized.
@swift-ci please smoke benchmark |
Build comment file:Optimized (O)Regression (3)
Improvement (6)
No Changes (325)
Unoptimized (Onone)Regression (10)
Improvement (12)
No Changes (312)
Hardware Overview
|
@slavapestov You meant this change when talking about |
@swiftix Yes, this is exactly what I meant, but of course also doing the extra step of removing the flag completely and constant-folding any places where it comes up. |
@slavapestov What about |
@swiftix No, -sil-serialize-vtables needs a bit more work. We don't want to serialize them in resilient builds, for instance. Let's discuss is in person next week. |
@swift-ci please smoke test OS X |
@swift-ci please smoke test |
1 similar comment
@swift-ci please smoke test |
@slavapestov OK, I pushed some updates. Now |
@huonw This PR touches a lot TBD-related things. I simply removed silSerializeWitnessTables flags everywhere. Is it OK? Or should I take any special care regarding the TBD implementation? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine; TBD is just passing that argument along to other subsystems, so it shouldn't need any special treatment.
@swift-ci please test |
1 similar comment
@swift-ci please test |
Build failed |
@swift-ci please smoke test compiler performance |
2 similar comments
@swift-ci please smoke test compiler performance |
@swift-ci please smoke test compiler performance |
@swift-ci Please test source compatibility |
1 similar comment
@swift-ci Please test source compatibility |
The functionality is always enabled now and there is no need to have a dedicated flag for it.
e905fb5
to
35a624b
Compare
@swift-ci please smoke test |
@swift-ci please test |
@swift-ci please smoke test |
Build failed |
@swift-ci please test |
@swift-ci please smoke benchmark |
1 similar comment
@swift-ci please smoke benchmark |
Build comment file:Optimized (O)Regression (2)
Improvement (5)
No Changes (327)
Unoptimized (Onone)Regression (9)
Improvement (8)
No Changes (317)
Hardware Overview
|
Now that we remove the [serialized] flag from functions after their early serialization, we can run another round of optimizations on them. Due to this change, it should be OK to serialize witness tables now because marking the witness methods [serialized] does not affect how well they can be optimized.