stdlib: switch string hashing on non-ObjC platforms to SipHash-1-3 #4621

Merged
merged 5 commits into from Sep 7, 2016

Projects

None yet

3 participants

@gribozavr
Collaborator
gribozavr commented Sep 3, 2016 edited

Part of rdar://problem/24109692.

@gribozavr
Collaborator

@swift-ci Please test

@swift-ci
Contributor
swift-ci commented Sep 3, 2016

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - f03c765
Test requested by - @gribozavr

@swift-ci
Contributor
swift-ci commented Sep 3, 2016

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - f03c765
Test requested by - @gribozavr

@gribozavr
Collaborator

@swift-ci Please test

@swift-ci
Contributor
swift-ci commented Sep 4, 2016

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - f03c765
Test requested by - @gribozavr

@gribozavr
Collaborator

@swift-ci Please test Linux platform

@swift-ci
Contributor
swift-ci commented Sep 4, 2016

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - c3a3cf9
Test requested by - @gribozavr

@gribozavr
Collaborator

@swift-ci Please test Linux platform

@swift-ci
Contributor
swift-ci commented Sep 4, 2016

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - c3a3cf9
Test requested by - @gribozavr

@gribozavr gribozavr referenced this pull request in apple/swift-corelibs-foundation Sep 4, 2016
Merged

Fix crash in NSValue.isEqual() when it is passed an NSNumber #619

@gribozavr
Collaborator

@swift-ci Please test Linux platform

@gribozavr
Collaborator

@swift-ci Please test OS X platform

@gribozavr
Collaborator

All dependencies have been merged. Retesting.

@swift-ci Please test

@gribozavr
Collaborator

@swift-ci Please test

@swift-ci
Contributor
swift-ci commented Sep 6, 2016

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 4f44c40
Test requested by - @gribozavr

@gribozavr
Collaborator

Linux failure is unrelated:

TestFoundation/TestNSOperationQueue.swift:63: error: TestNSOperationQueue.test_OperationPriorities : XCTAssertEqual failed: ("Operation1 executed") is not equal to ("Operation3 executed") - 
@gribozavr
Collaborator

@swift-ci Please test

@gribozavr gribozavr changed the title from [don't merge] Switch string hashing on non-ObjC platforms to Siphash-1-3 to Switch string hashing on non-ObjC platforms to Siphash-1-3 Sep 7, 2016
@gribozavr gribozavr changed the title from Switch string hashing on non-ObjC platforms to Siphash-1-3 to Switch string hashing on non-ObjC platforms to SipHash-1-3 Sep 7, 2016
@gribozavr
Collaborator

@swift-ci Please smoke test OS X platform

@gribozavr gribozavr changed the title from Switch string hashing on non-ObjC platforms to SipHash-1-3 to stdlib: switch string hashing on non-ObjC platforms to SipHash-1-3 Sep 7, 2016
@gribozavr gribozavr merged commit bfd5942 into master Sep 7, 2016

3 checks passed

Swift Test Linux Platform Build finished.
Details
Swift Test Linux Platform (smoke test)
Details
Swift Test OS X Platform (smoke test)
Details
@gribozavr gribozavr deleted the siphash branch Sep 7, 2016
@swift-ci
Contributor
swift-ci commented Sep 7, 2016

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 4f44c40
Test requested by - @gribozavr

@jckarter

Is there a reason this has to be statically constructed? Does computing it lazily have noticeable overhead, or security concerns?

Collaborator

I expect almost every Swift program to use Dictionary extensively, so this variable will be initialized in every Swift program anyway. It is not uncommon to use Dictionary or Set early in the program run (the same way NSDictionary is used everywhere). I also expect this variable to be accessed frequently -- in future, once for every hash value compted. Thus, I think it is reasonable to perform eager initialization of this variable to remove checks everywhere else in the process.

Member

Makes sense. It might be a good idea to use open and read so we don't otherwise have to warm up stdio right at process start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment