Skip to content
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

wrk improvements #174

Merged
merged 14 commits into from
Apr 9, 2016
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
7 changes: 7 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,10 @@ script:
- swift build
- swift build --configuration release
- swift test
- sudo apt-get install -y build-essential libssl-dev git
- git clone https://github.com/wg/wrk.git
- cd wrk
- make
- sudo cp wrk /usr/local/bin
- .build/release/Performance --port=8523 --env=production &
- wrk -d 10 -t 4 -c 128 http://127.0.0.1:8523/plaintext
11 changes: 9 additions & 2 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ let package = Package(
.Package(url: "https://github.com/Zewo/JSON.git", majorVersion: 0, minor: 4),

//Swift wrapper around Sockets, used for built-in HTTP server
.Package(url: "https://github.com/ketzusaka/Hummingbird.git", majorVersion: 1, minor: 1),
.Package(url: "https://github.com/qutheory/Hummingbird.git", majorVersion: 2, minor: 0),

//SHA2 + HMAC hashing. Used by the core to create session identifiers.
.Package(url: "https://github.com/CryptoKitten/HMAC.git", majorVersion: 0, minor: 3),
.Package(url: "https://github.com/CryptoKitten/SHA2.git", majorVersion: 0, minor: 1)
],
exclude: [
"XcodeProject",
"Sources/Generator"
],
targets: [
Target(
Expand All @@ -30,7 +31,13 @@ let package = Package(
]
),
Target(
name: "VaporDev",
name: "Development",
dependencies: [
.Target(name: "Vapor")
]
),
Target(
name: "Performance",
dependencies: [
.Target(name: "Vapor")
]
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
9 changes: 9 additions & 0 deletions Sources/Performance/main.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import Vapor

let app = Application()

app.get("plaintext") { request in
return "Hello, world!"
}

app.start()
21 changes: 15 additions & 6 deletions Sources/Vapor/Server/HTTPStream.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,24 @@ extension HTTPStream {

func receiveLine() throws -> String {
var line: String = ""
func append(byte: Byte) {
guard byte >= minimumValidAsciiCharacter else { return }

func append(byte: Byte?) {
guard let byte = byte else {
return
}
guard byte >= minimumValidAsciiCharacter else {
return
}

line.append(Character(byte))
}

while
let next = try receiveByte()
where next != newLine
&& !closed {
var next: Byte?
while next != newLine && !closed {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I understand the difference here., am I missing something?

Copy link
Member Author

Choose a reason for hiding this comment

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

Before, if the first byte(s) received were nil, it would assume end of line and return whatever it had. It looks like some requests (especially wrk) start with an empty byte. So the empty byte came in, this skipped and returned an empty string, the request line got parsed as invalid, then it was all wonky after that.

Now it will only assumes the line is done when it gets a new line character, which seems to be more technically valid, and also totally fixed the InvalidComponents issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ketzusaka Here

repeat {
next = try receiveByte()
} while next == nil
Copy link
Member

Choose a reason for hiding this comment

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

Ahhh, is this it where it'll keep trying until it has value? Would it be worth receiveByte doing some of the heavy lifting there, or does that not make sense.

Copy link
Member Author

Choose a reason for hiding this comment

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

True...receiveByte could spin until it gets a byte.... that might be more logical.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ketzusaka And here.


append(next)
}

Expand Down
62 changes: 28 additions & 34 deletions Sources/Vapor/Server/HTTPStreamServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ class HTTPStreamServer<StreamType: HTTPStream>: Server {
var delegate: Responder!

func serve(responder: Responder, on host: String, at port: Int) throws {
halt()
self.delegate = responder

try stream.bind(to: host, on: port)
Expand All @@ -40,40 +39,35 @@ class HTTPStreamServer<StreamType: HTTPStream>: Server {
}

private func handle(socket: HTTPStream) {
do {
try Background {
var keepAlive = false
repeat {
let request: Request
do {
request = try socket.receive()
} catch {
Log.error("Error receiving request: \(error)")
return
}

let response: Response
do {
response = try self.delegate.respond(request)
} catch {
Log.error("Error parsing response: \(error)")
return
}

do {
try socket.send(response, keepAlive: keepAlive)
} catch {
Log.error("Error sending response: \(error)")
}

keepAlive = request.supportsKeepAlive
} while keepAlive && !socket.closed

socket.close()
var keepAlive = false

repeat {
Copy link
Member

Choose a reason for hiding this comment

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

Does this not need to be dispatched to a new thread?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, Hummingbird dispatches it for us. That's where the huge speed gain we got was.

Copy link
Member Author

Choose a reason for hiding this comment

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

From Hummingbird:

_ = try Strand {
    connectionHandler(Socket(socketDescriptor: requestDescriptor))
}

Copy link
Member

Choose a reason for hiding this comment

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

Awesome, that's a really good catch! Half the dispatching is probably for the best 😄

let request: Request
do {
request = try socket.receive()
} catch {
Log.error("Error receiving request: \(error)")
return
}
} catch {
Log.error("Error accepting request in background: \(error)")
}

let response: Response
do {
response = try self.delegate.respond(request)
} catch {
Log.error("Error parsing response: \(error)")
return
}

do {
try socket.send(response, keepAlive: keepAlive)
} catch {
Log.error("Error sending response: \(error)")
}

keepAlive = request.supportsKeepAlive
} while keepAlive && !socket.closed

socket.close()
}

}
Expand Down
25 changes: 0 additions & 25 deletions Sources/Vapor/Server/Vapor+Hummingbird.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,29 +20,4 @@ extension Hummingbird.Socket: HTTPStream {
try listen(pendingConnectionBacklog: 100)
}

enum Error: ErrorProtocol {
case Unsupported
}

public var closed: Bool {
return false
}

public func close() -> Bool {
return false
}

public func receive(max maxBytes: Int) throws -> Data {
let bytes: [Byte] = try self.receive(maximumBytes: maxBytes) ?? []
return Data(bytes)
}

public func send(data: Data) throws {
try self.send(data.bytes)
}

public func flush() throws {
throw Error.Unsupported
}

}
4 changes: 2 additions & 2 deletions Tests/Vapor/ConfigTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ class ConfigTests: XCTestCase {
}

#if Xcode
let workDir = "/Users/tanner/Developer/vapor/vapor/Sources/VaporDev/"
let workDir = "/Users/tanner/Developer/vapor/vapor/Sources/Development/"
Copy link
Member

Choose a reason for hiding this comment

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

Not sure, but is it possible to generalize this somehow? Do environment variables like $(SRC_ROOT) work in code somehow

Copy link
Member Author

Choose a reason for hiding this comment

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

I remember trying really hard to make it work to no avail. Maybe I should add a comment explaining why it needs to be there.

#else
let workDir = "Sources/VaporDev/"
let workDir = "Sources/Development/"
Copy link
Member

Choose a reason for hiding this comment

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

👍 I like this name change.

Copy link
Member Author

Choose a reason for hiding this comment

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

I kept getting confused with my projects that used Vapor and starting VaporDev instead of App >_< Figured this might be more clear.

#endif

func testSimple() {
Expand Down
Loading