Skip to content

Document and explain test #225

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

Closed
wants to merge 1 commit into from
Closed

Document and explain test #225

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 35 additions & 14 deletions validation-test/stdlib/StringSlicesConcurrentAppend.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,17 @@ import Darwin
import Glibc
#endif

// Swift.String has an optimization that allows us to append to a shared string
// buffer. Make sure that it works correctly when two threads try to append to
// different non-shared strings that point to the same shared buffer.

var StringTestSuite = TestSuite("String")

// Specific to test:
// We need to know what the internal ID of the buffer is so we can check that only
// one thread owns a buffer.
//
// TODO: explain why we're looking for a specific capacity.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is redundant, this interaction is explained later in the code.

extension String {
var bufferID: UInt {
return unsafeBitCast(_core._owner, UInt.self)
Expand All @@ -20,18 +29,16 @@ extension String {
}
}

// Swift.String has an optimization that allows us to append to a shared string
// buffer. Make sure that it works correctly when two threads try to append to
// different non-shared strings that point to the same shared buffer.

// OwnerThread is the thread we expect to end up owning sharedString
// OtherThread is the thread we expect to *not* own sharedString
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This naming change does not affect the test setup: depending on thread progress, either thread can end up owning the sharedString.

enum ThreadID {
case Leader
case Follower
case OwnerThread
case OtherThread
}

var barrierVar: UnsafeMutablePointer<_stdlib_pthread_barrier_t> = nil
var sharedString: String = ""
var followerString: String = ""
var otherThreadString: String = ""

func barrier() {
var ret = _stdlib_pthread_barrier_wait(barrierVar)
Expand All @@ -41,7 +48,7 @@ func barrier() {
func sliceConcurrentAppendThread(tid: ThreadID) {
for i in 0..<100 {
barrier()
if tid == .Leader {
if tid == .OwnerThread {
// Get a fresh buffer.
sharedString = ""
sharedString.appendContentsOf("abc")
Expand All @@ -52,12 +59,15 @@ func sliceConcurrentAppendThread(tid: ThreadID) {
barrier()

// Get a private string.
// we *should* be able to change this as we wish without modifying the underlying string.
var privateString = sharedString

barrier()

// Append to the private string.
if tid == .Leader {
// This should invoke a deep copy somewhere along the line,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swift does not use or define "deep copy", so I don't know what it means in this context.

// but NOT change the underlying sharedString buffer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but no, the buffer of sharedString will be changed by one of the threads. Another thread will allocate its own thread. The reason why the user-visible value of sharedString won't change is that its (not shared) length will not change.

if tid == .OwnerThread {
privateString.appendContentsOf("def")
} else {
privateString.appendContentsOf("ghi")
Expand All @@ -66,22 +76,33 @@ func sliceConcurrentAppendThread(tid: ThreadID) {
barrier()

// Verify that contents look good.
if tid == .Leader {
// if our current thread is the OwnerThread, it should be "abcdef"
// if our current thread is NOT OwnerThread, it should be "abcghi"
// However, sharedString should NOT be changed, and should be "abc" no matter what.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment duplicates code, I don't see how it explains anything.

if tid == .OwnerThread {
expectEqual("abcdef", privateString)
} else {
expectEqual("abcghi", privateString)
}

// sharedString should not have been touched.
expectEqual("abc", sharedString)

// Verify that only one thread took ownership of the buffer.
if tid == .Follower {
followerString = privateString
if tid == .OtherThread {
otherThreadString = privateString
}
barrier()
if tid == .Leader {
if tid == .OwnerThread {
// We expect that
// privateString.bufferID == sharedString.bufferID
// - OR -
// otherThreadString.bufferID == sharedString.bufferID
//
// but not both.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this comment duplicates code.

expectTrue(
(privateString.bufferID == sharedString.bufferID) !=
(followerString.bufferID == sharedString.bufferID))
(otherThreadString.bufferID == sharedString.bufferID))
}
}
}
Expand Down