Re: [swift-dev] Debugging performance regressions?

2017-08-09 Thread Mark Lacey via swift-dev


> On Aug 9, 2017, at 11:33 AM, Arnold Schwaighofer  
> wrote:
> 
> Mark do you have suggestions for Dave?

I’d suggest something similar to what Pavel said. My usual first step is to 
look at the output of -Xfrontend -debug-constraints, which in cases like this 
tends to be so verbose that you have to kill the compile after a little while 
and a few megabytes of output and start reviewing that output. Key things to 
look for are “(assuming”, which is printed for each disjunction choice, and 
“(trying” which is printed before each attempt at trying a type binding.

I took a quick look at this and based on the before/after output it looks like 
perhaps an optimization that picks favored overloads is no longer firing after 
your PR. As a result, rather than trying and successfully solving using integer 
operations for the arithmetic and stopping there, we exponentially attempt all 
combinations of all overloads of those operators. It would be very interesting 
to understand why that is since based on the description I would not expect 
your PR to have an impact on that.

Mark

> 
>> On Aug 9, 2017, at 11:22 AM, David Zarzycki via swift-dev 
>> > wrote:
>> 
>> Any tips on how to debug performance regressions?
>> 
>> And as aside, if this test is critical enough to cause a revert, why isn’t 
>> it in the public repo?
>> 
> 
> This is part of a larger internal test.
> 
>> 
>>> On Aug 9, 2017, at 14:04, Arnold Schwaighofer >> > wrote:
>>> 
>>> Hi Dave,
>>> I have reverted this commit because it broke internal bots.
>>> 
>>> import Foundation
>>> class P {
>>> var x : Int = 0
>>> var y : Int = 1
>>> }
>>> 
>>> let dist : (P, P) -> Double = {
>>>   (p : P, s : P)  -> Double in
>>> sqrt(Double((p.x-s.x)*(p.x-s.x) + (p.y-s.y)*(p.y-s.y)))
>>> }
>>> fails now with expression to complex.
>>> 
>>> —
>>> You are receiving this because you authored the thread.
>>> Reply to this email directly, view it on GitHub 
>>> , or mute 
>>> the thread 
>>> .
>>> 
>> 
>> ___
>> swift-dev mailing list
>> swift-dev@swift.org 
>> https://lists.swift.org/mailman/listinfo/swift-dev
> 

___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


Re: [swift-dev] Debugging performance regressions?

2017-08-09 Thread Michael Gottesman via swift-dev

> On Aug 9, 2017, at 11:34 AM, Arnold Schwaighofer via swift-dev 
>  wrote:
> 
> Mark do you have suggestions for Dave?

Also, whatever comes from this, can we document in updating the compiler.

> 
>> On Aug 9, 2017, at 11:22 AM, David Zarzycki via swift-dev 
>> > wrote:
>> 
>> Any tips on how to debug performance regressions?
>> 
>> And as aside, if this test is critical enough to cause a revert, why isn’t 
>> it in the public repo?
>> 
> 
> This is part of a larger internal test.
> 
>> 
>>> On Aug 9, 2017, at 14:04, Arnold Schwaighofer >> > wrote:
>>> 
>>> Hi Dave,
>>> I have reverted this commit because it broke internal bots.
>>> 
>>> import Foundation
>>> class P {
>>> var x : Int = 0
>>> var y : Int = 1
>>> }
>>> 
>>> let dist : (P, P) -> Double = {
>>>   (p : P, s : P)  -> Double in
>>> sqrt(Double((p.x-s.x)*(p.x-s.x) + (p.y-s.y)*(p.y-s.y)))
>>> }
>>> fails now with expression to complex.
>>> 
>>> —
>>> You are receiving this because you authored the thread.
>>> Reply to this email directly, view it on GitHub 
>>> , or mute 
>>> the thread 
>>> .
>>> 
>> 
>> ___
>> swift-dev mailing list
>> swift-dev@swift.org 
>> https://lists.swift.org/mailman/listinfo/swift-dev
> 
> ___
> swift-dev mailing list
> swift-dev@swift.org 
> https://lists.swift.org/mailman/listinfo/swift-dev 
> 
___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


Re: [swift-dev] Debugging performance regressions?

2017-08-09 Thread Arnold Schwaighofer via swift-dev
Mark do you have suggestions for Dave?

> On Aug 9, 2017, at 11:22 AM, David Zarzycki via swift-dev 
>  wrote:
> 
> Any tips on how to debug performance regressions?
> 
> And as aside, if this test is critical enough to cause a revert, why isn’t it 
> in the public repo?
> 

This is part of a larger internal test.

> 
>> On Aug 9, 2017, at 14:04, Arnold Schwaighofer > > wrote:
>> 
>> Hi Dave,
>> I have reverted this commit because it broke internal bots.
>> 
>> import Foundation
>> class P {
>> var x : Int = 0
>> var y : Int = 1
>> }
>> 
>> let dist : (P, P) -> Double = {
>>   (p : P, s : P)  -> Double in
>> sqrt(Double((p.x-s.x)*(p.x-s.x) + (p.y-s.y)*(p.y-s.y)))
>> }
>> fails now with expression to complex.
>> 
>> —
>> You are receiving this because you authored the thread.
>> Reply to this email directly, view it on GitHub 
>> , or mute 
>> the thread 
>> .
>> 
> 
> ___
> swift-dev mailing list
> swift-dev@swift.org
> https://lists.swift.org/mailman/listinfo/swift-dev

___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


Re: [swift-dev] Debugging performance regressions?

2017-08-09 Thread Pavel Yaskevich via swift-dev
Hi David,

   What I usually do is - lldb with debug build, or debug build REPL with
":constraints debug on" which is going to print what type checker is doing
   and attach using lldb or Xcode.

   This example definitely belongs in the repo, I will add it right away.

Best Regards, Pavel.


On Wed, Aug 9, 2017 at 11:22 AM, David Zarzycki via swift-dev <
swift-dev@swift.org> wrote:

> Any tips on how to debug performance regressions?
>
> And as aside, if this test is critical enough to cause a revert, why isn’t
> it in the public repo?
>
>
> On Aug 9, 2017, at 14:04, Arnold Schwaighofer 
> wrote:
>
> Hi Dave,
> I have reverted this commit because it broke internal bots.
>
> import Foundation
> class P {
> var x : Int = 0
> var y : Int = 1
> }
>
> let dist : (P, P) -> Double = {
>   (p : P, s : P)  -> Double in
> sqrt(Double((p.x-s.x)*(p.x-s.x) + (p.y-s.y)*(p.y-s.y)))
> }
>
> fails now with expression to complex.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> , or mute
> the thread
> 
> .
>
>
>
> ___
> swift-dev mailing list
> swift-dev@swift.org
> https://lists.swift.org/mailman/listinfo/swift-dev
>
>
___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


[swift-dev] Debugging performance regressions?

2017-08-09 Thread David Zarzycki via swift-dev
Any tips on how to debug performance regressions?

And as aside, if this test is critical enough to cause a revert, why isn’t it 
in the public repo?


> On Aug 9, 2017, at 14:04, Arnold Schwaighofer  
> wrote:
> 
> Hi Dave,
> I have reverted this commit because it broke internal bots.
> 
> import Foundation
> class P {
> var x : Int = 0
> var y : Int = 1
> }
> 
> let dist : (P, P) -> Double = {
>   (p : P, s : P)  -> Double in
> sqrt(Double((p.x-s.x)*(p.x-s.x) + (p.y-s.y)*(p.y-s.y)))
> }
> fails now with expression to complex.
> 
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub 
> , or mute 
> the thread 
> .
> 

___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


Re: [swift-dev] 107 lines of "dead" code in ConstraintSystem::performMemberLookup?

2017-08-09 Thread Michael Gottesman via swift-dev
I think there are docs on this in the main README or in a CONTRIBUTING file.

Mishal (+CC) knows for sure.

Michael

> On Aug 9, 2017, at 5:41 AM, David Zarzycki via swift-dev 
>  wrote:
> 
> Hi Slava,
> 
> Thanks! I appreciate the suggestion to run tests, but what I’ve tried in the 
> past, @swift-ci ignored me. Is there a whitelist for who can request that 
> tests be run? That would make sense from a security perspective.
> 
> Dave
> 
> 
> 
>> On Aug 8, 2017, at 23:48, Slava Pestov > > wrote:
>> 
>> I see you’re two steps ahead of me already: 
>> https://github.com/apple/swift/pull/11397 
>> 
>> 
>> This looks good, nice catch!
>> 
>> Slava
>> 
>>> On Aug 8, 2017, at 8:45 PM, Slava Pestov >> > wrote:
>>> 
>>> 
 On Aug 8, 2017, at 3:34 PM, David Zarzycki via swift-dev 
 > wrote:
 
 In ConstraintSystem::performMemberLookup(), constructors with “simple” 
 names have a dedicated lookup path. In contrast, constructors with 
 compound names are handled by the normal lookup. If I delete this code and 
 let the normal lookup path handle both simple and compound named 
 constructors, I find that all 10,214 validation tests pass on my machine 
 (albeit with slightly different error messages in three test files).
>>> 
>>> I would suggest running the source compatibility test suite also (see “pull 
>>> request testing” in https://swift.org/source-compatibility/ 
>>> ), but it is quite possible the 
>>> code is indeed unnecessary.
>>> 
 
 Is the test suite missing a test for this code path and if so, what? Or 
 should it be scheduled for deletion after identical error messages can be 
 generated by the normal lookup path?
>>> 
>>> Are the new error messages worse or just different? If the latter there’s 
>>> really no requirement to keep them identical.
>>> 
>>> Slava
>>> 
 
 Dave
 ___
 swift-dev mailing list
 swift-dev@swift.org 
 https://lists.swift.org/mailman/listinfo/swift-dev 
 
>>> 
>> 
> 
> ___
> swift-dev mailing list
> swift-dev@swift.org
> https://lists.swift.org/mailman/listinfo/swift-dev

___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


Re: [swift-corelibs-dev] NSString.range(of:options:range:locale) incompatibility

2017-08-09 Thread Tony Parker via swift-corelibs-dev
Hi Ian,

I don’t think Max has put up that PR against the swift-corelibs-foundation 
implementation, but we should.

- Tony

> On Aug 9, 2017, at 9:04 AM, Ian Partridge via swift-corelibs-dev 
>  wrote:
> 
> I've noticed this current difference in behaviour.
> 
> Darwin:
> 
> $ swift
> Welcome to Apple Swift version 4.0-dev (LLVM 2dedb62a0b, Clang b9d76a314c, 
> Swift 6b4756bd93). Type :help for assistance.
>   1> import Foundation
>   2> "abc".range(of: "a")!.lowerBound..<"abc".range(of: "a")!.upperBound
> $R0: Range = {
>   lowerBound = {
> _compoundOffset = 0
> _cache = utf16
>   }
>   upperBound = {
> _compoundOffset = 4
> _cache = utf16
>   }
> }
> 
> Linux:
> 
> $ swift
> Welcome to Swift version 4.0-dev (LLVM 2dedb62a0b, Clang b9d76a314c, Swift 
> 6b4756bd93). Type :help for assistance.
>   1> import Foundation 
>   2> "abc".range(of: "a")!.lowerBound..<"abc".range(of: "a")!.upperBound 
> $R0: CountableRange = {
>   lowerBound = {
> _compoundOffset = 0
> _cache = utf16
>   }
>   upperBound = {
> _compoundOffset = 4
> _cache = utf16
>   }
> }
> 
> I think this might be because some changes made in the overlay 
> (NSStringAPI.swift) haven't been ported across to swift-corelibs-foundation:
> 
> https://github.com/apple/swift/commit/5f1c3f702189f8197e297479d464dc454330be80
>  
> 
> 
> Is my suspicion right, and is there any work in progress to bring this into 
> sync before Swift 4?
> 
> -- 
> Ian Partridge
> ___
> swift-corelibs-dev mailing list
> swift-corelibs-dev@swift.org
> https://lists.swift.org/mailman/listinfo/swift-corelibs-dev

___
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev


Re: [swift-dev] 107 lines of "dead" code in ConstraintSystem::performMemberLookup?

2017-08-09 Thread David Zarzycki via swift-dev
Hi Slava,

Thanks! I appreciate the suggestion to run tests, but what I’ve tried in the 
past, @swift-ci ignored me. Is there a whitelist for who can request that tests 
be run? That would make sense from a security perspective.

Dave



> On Aug 8, 2017, at 23:48, Slava Pestov  wrote:
> 
> I see you’re two steps ahead of me already: 
> https://github.com/apple/swift/pull/11397 
> 
> 
> This looks good, nice catch!
> 
> Slava
> 
>> On Aug 8, 2017, at 8:45 PM, Slava Pestov > > wrote:
>> 
>> 
>>> On Aug 8, 2017, at 3:34 PM, David Zarzycki via swift-dev 
>>> > wrote:
>>> 
>>> In ConstraintSystem::performMemberLookup(), constructors with “simple” 
>>> names have a dedicated lookup path. In contrast, constructors with compound 
>>> names are handled by the normal lookup. If I delete this code and let the 
>>> normal lookup path handle both simple and compound named constructors, I 
>>> find that all 10,214 validation tests pass on my machine (albeit with 
>>> slightly different error messages in three test files).
>> 
>> I would suggest running the source compatibility test suite also (see “pull 
>> request testing” in https://swift.org/source-compatibility/ 
>> ), but it is quite possible the 
>> code is indeed unnecessary.
>> 
>>> 
>>> Is the test suite missing a test for this code path and if so, what? Or 
>>> should it be scheduled for deletion after identical error messages can be 
>>> generated by the normal lookup path?
>> 
>> Are the new error messages worse or just different? If the latter there’s 
>> really no requirement to keep them identical.
>> 
>> Slava
>> 
>>> 
>>> Dave
>>> ___
>>> swift-dev mailing list
>>> swift-dev@swift.org 
>>> https://lists.swift.org/mailman/listinfo/swift-dev
>> 
> 

___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev