[swift-evolution] [swift-evolution-announce] [Review] Remove C-style for-loops with conditions and incrementers
Nadav Rotem
nrotem at apple.com
Fri Dec 11 12:28:55 CST 2015
> On Dec 11, 2015, at 9:09 AM, Joe Groff via swift-evolution <swift-evolution at swift.org> wrote:
>
>>
>> On Dec 10, 2015, at 5:17 PM, David Owens II via swift-evolution <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> wrote:
>>
>> Review of SE-0007
>>
>> * What is your evaluation of the proposal?
>>
>> I am against this change. I’ve seen no consideration of the performance implications that this change brings about. Also, many of the proposed alternatives tend towards using defer as the mechanism to provide consistent increments/decrements. However, this is problematic around the boundary edges as break and throw are not mechanisms that can halt the execution of the defer statement. So unlike the c-style loop that would not perform that code, the alternatives would.
In the general case for-each loops are as efficient as c-style for loops. For example, the memset function below is optimized to something that is equivalent to the performance of C’s memset (no bounds checks, no overflow checks, vectorized, unrolled, etc).
I reverted the changes in the standard library because they exposed a problem in a different part of the code. The code for for-each is larger than c-style loops before it is optimized. According to Arnold, this changed the inlining decision and caused an extra retain/release pair, that slowed down String.
func memset(inout a: [Int], _ c: Int) {
for i in 0..<a.count {
a[i] = c
}
}
>>
>> * Is the problem being addressed significant enough to warrant a change to Swift?
>>
>> No; it offers no alternative solution to maintain the performance that a c-style for-loop ensures. There is no general pattern than can be used to satisfy a c-style loop’s behavior; each proposed alternative has different issues.
>>
>> * Does this proposal fit well with the feel and direction of Swift?
>>
>> No, it limits the language’s ability to create concise and legible code when the situation does arise for a c-style loop.
>>
>> This is the alternative to create a loop that matches the same functionality, including preventing the leaking of loop-variables.
>>
>> var sum = 0
>> for var i = 1000, j = 2000; i > 0 && j > 0; i -= 1, j -= 2 {
>> if i % 2 == 0 { continue }
>> sum += 1
>> }
>> print(sum)
>>
>> // versus
>>
>> var sum = 0
>>
>> if true { // some arbitrary scope mechanism is needed to contain i and j
>> var i = 1000, j = 2000
>> while i > 0 && j > 0 {
>> defer {
>> i -= 1
>> j -= 2
>> }
>>
>> if i % 2 == 0 { continue }
>> sum += 1
>> }
>> }
>>
>> print(sum)
>>
>> To me, that is not a fair trade-off; maybe if the defer pattern could be used at all times and the leaking of the loop-variables was considered ok. There are some ways to consolidate the code so that it takes up less visual space, but I don’t do that for other constructs, so I don’t think that is a valid thing to try and do here. Also, the use of defer in this context is a somewhat of a trick that I’m not sure is even a good pattern to promote as it can lead to boundary-case crashes.
>>
>> This is unsafe code:
>>
>> var sum = 0
>> var i: UInt = 1000
>> while i >= 0 {
>> defer {
>> i -= 1
>> }
>> if i == 0 {
>> print("zero has been reached!")
>> break
>> }
>>
>> if i % 2 == 0 { continue }
>> sum += 1
>> }
>>
>> The defer statement is still executed, so now I need to come up with a different pattern to handle this type of problem.
>>
>> * If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
>>
>> n/a
>>
>> * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
>>
>> A few hours. I’ve read the proposal multiple times and followed the mail threads. I’ve also done some basic performance analysis (attached mail) on the problem for some of the “improved” versions of the loop construct.
>
> Performance is definitely a consideration; we already reverted a pull request that remove C-style fors from the standard library. I believe Andy is currently looking into where the regressions come from. stride(...) performing poorly seems like something we should fix regardless, since that's arguably the idiomatic way to write such loops and ought to perform well. I agree we should investigate ways to ensure common loops over ranges or strides perform reasonably at -Onone if we move forward with this.
>
> -Joe
>
>> -David
>>
>> <SE-0007 Remove C-style for-loops with conditions and incrementers - Performance Consideration.eml>
>>
>>> On Dec 7, 2015, at 12:44 PM, Douglas Gregor <dgregor at apple.com <mailto:dgregor at apple.com>> wrote:
>>>
>>> Hello Swift community,
>>>
>>> The review of "Remove C-style for-loops with conditions and incrementers” begins now and runs through Thursday, December 10th. The proposal is available here:
>>>
>>> https://github.com/apple/swift-evolution/blob/master/proposals/0007-remove-c-style-for-loops.md <https://github.com/apple/swift-evolution/blob/master/proposals/0007-remove-c-style-for-loops.md>
>>>
>>> Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at
>>>
>>> https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>
>>>
>>> or, if you would like to keep your feedback private, directly to the review manager.
>>>
>>> What goes into a review?
>>>
>>> The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:
>>>
>>> * What is your evaluation of the proposal?
>>> * Is the problem being addressed significant enough to warrant a change to Swift?
>>> * Does this proposal fit well with the feel and direction of Swift?
>>> * If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
>>> * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
>>>
>>> More information about the Swift evolution process is available at
>>>
>>> https://github.com/apple/swift-evolution/blob/master/process.md <https://github.com/apple/swift-evolution/blob/master/process.md>
>>>
>>> Cheers,
>>> Doug Gregor
>>> Review Manager
>>>
>>>
>>> _______________________________________________
>>> swift-evolution-announce mailing list
>>> swift-evolution-announce at swift.org <mailto:swift-evolution-announce at swift.org>
>>> https://lists.swift.org/mailman/listinfo/swift-evolution-announce <https://lists.swift.org/mailman/listinfo/swift-evolution-announce>
>>
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution at swift.org <mailto:swift-evolution at swift.org>
>> https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org <mailto:swift-evolution at swift.org>
> https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20151211/56bfdb04/attachment.html>
More information about the swift-evolution
mailing list