Skip to content

Commit

Permalink
Improve performance of routing inbound RPCs (#924)
Browse files Browse the repository at this point in the history
Motivation:

Right now the server burns quite a lot of CPU time initially working out
which RPC handlers to use when it wants to dispatch an inbound RPC. One
chunk of time here involves processing the inbound request URI to work
out the appropriate method name.

The costs here come in a few places, but the biggest thing blocking a
performance improvement is `String.components(separatedBy:)`. This
function is secretly an NSString function. The result is that the mere
act of calling this function forces us to transform the UTF8 Swift
String into a UTF16 NSString, split on the slash, and then allocate an
NSArray into which we will re-encode each of the components as Swift
Strings (transforming back from UTF16 to UTF8). We then throw away most
of the results, and pass the string into the CallHandlerProvider.

This is way too much work. At the very least we should swap
`String.components(separatedBy:)` for `String.split`, which does not
require Foundation. This avoids the extra `String` constructions and any
other mess.

What would be even better, though, is to not work in String space at all
but instead in Substring space. That is, if we can avoid using Strings
inside GRPCServerRequestRoutingHandler at all in favour of Substrings,
we can avoid ever needing to create a String to dispatch an RPC. We'll
instead look up the RPCs using a Substring, and then dispatch into the
individual CallHandlerProvider with that substring.

This avoids any need to allocate at all on the entire codepath except
for the Array returned by `split`. For now we'll tolerate that, and if
it really becomes too big of a performance problem we can try to
investigate an alternative approach there instead (such as working in
UTF-8 space directly).

Modifications:

- Replace String.components(separatedBy:) with String.split.
- Update GRPCServerRequestRoutingHandler to compute on Substring, not
  String.
- Update codegen to generate conforming code.

Results:

Improved performance on RPC dispatch, about a 4% win.
  • Loading branch information
Lukasa authored Aug 7, 2020
1 parent 46d1a29 commit 6158fad
Show file tree
Hide file tree
Showing 15 changed files with 39 additions and 40 deletions.
4 changes: 2 additions & 2 deletions Sources/Examples/Echo/Model/echo.grpc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -271,11 +271,11 @@ public protocol Echo_EchoProvider: CallHandlerProvider {
}

extension Echo_EchoProvider {
public var serviceName: String { return "echo.Echo" }
public var serviceName: Substring { return "echo.Echo" }

/// Determines, calls and returns the appropriate request handler, depending on the request's method.
/// Returns nil for methods not handled by this service.
public func handleMethod(_ methodName: String, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
public func handleMethod(_ methodName: Substring, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
switch methodName {
case "Get":
return CallHandlerFactory.makeUnary(callHandlerContext: callHandlerContext) { context in
Expand Down
4 changes: 2 additions & 2 deletions Sources/Examples/HelloWorld/Model/helloworld.grpc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ public protocol Helloworld_GreeterProvider: CallHandlerProvider {
}

extension Helloworld_GreeterProvider {
public var serviceName: String { return "helloworld.Greeter" }
public var serviceName: Substring { return "helloworld.Greeter" }

/// Determines, calls and returns the appropriate request handler, depending on the request's method.
/// Returns nil for methods not handled by this service.
public func handleMethod(_ methodName: String, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
public func handleMethod(_ methodName: Substring, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
switch methodName {
case "SayHello":
return CallHandlerFactory.makeUnary(callHandlerContext: callHandlerContext) { context in
Expand Down
4 changes: 2 additions & 2 deletions Sources/Examples/RouteGuide/Model/route_guide.grpc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,11 @@ public protocol Routeguide_RouteGuideProvider: CallHandlerProvider {
}

extension Routeguide_RouteGuideProvider {
public var serviceName: String { return "routeguide.RouteGuide" }
public var serviceName: Substring { return "routeguide.RouteGuide" }

/// Determines, calls and returns the appropriate request handler, depending on the request's method.
/// Returns nil for methods not handled by this service.
public func handleMethod(_ methodName: String, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
public func handleMethod(_ methodName: Substring, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
switch methodName {
case "GetFeature":
return CallHandlerFactory.makeUnary(callHandlerContext: callHandlerContext) { context in
Expand Down
17 changes: 8 additions & 9 deletions Sources/GRPC/GRPCServerRequestRoutingHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import Foundation
import SwiftProtobuf
import NIO
import NIOHTTP1
Expand All @@ -31,11 +30,11 @@ public protocol CallHandlerProvider: class {
/// The name of the service this object is providing methods for, including the package path.
///
/// - Example: "io.grpc.Echo.EchoService"
var serviceName: String { get }
var serviceName: Substring { get }

/// Determines, calls and returns the appropriate request handler (`GRPCCallHandler`), depending on the request's
/// method. Returns nil for methods not handled by this service.
func handleMethod(_ methodName: String, callHandlerContext: CallHandlerContext) -> GRPCCallHandler?
func handleMethod(_ methodName: Substring, callHandlerContext: CallHandlerContext) -> GRPCCallHandler?
}

// This is public because it will be passed into generated code, all members are `internal` because
Expand All @@ -57,7 +56,7 @@ public struct CallHandlerContext {
/// from the pipeline.
public final class GRPCServerRequestRoutingHandler {
private let logger: Logger
private let servicesByName: [String: CallHandlerProvider]
private let servicesByName: [Substring: CallHandlerProvider]
private let encoding: ServerMessageEncoding
private weak var errorDelegate: ServerErrorDelegate?

Expand All @@ -69,7 +68,7 @@ public final class GRPCServerRequestRoutingHandler {
private var state: State = .notConfigured

public init(
servicesByName: [String: CallHandlerProvider],
servicesByName: [Substring: CallHandlerProvider],
encoding: ServerMessageEncoding,
errorDelegate: ServerErrorDelegate?,
logger: Logger
Expand Down Expand Up @@ -219,17 +218,17 @@ extension GRPCServerRequestRoutingHandler: ChannelInboundHandler, RemovableChann
// `CallHandlerProvider`s should provide the service name including the package name.
// - uriComponents[2]: method name.
self.logger.debug("making call handler", metadata: ["path": "\(requestHead.uri)"])
let uriComponents = requestHead.uri.components(separatedBy: "/")
let uriComponents = requestHead.uri.split(separator: "/")

let context = CallHandlerContext(
errorDelegate: self.errorDelegate,
logger: self.logger,
encoding: self.encoding
)

guard uriComponents.count >= 3 && uriComponents[0].isEmpty,
let providerForServiceName = servicesByName[uriComponents[1]],
let callHandler = providerForServiceName.handleMethod(uriComponents[2], callHandlerContext: context) else {
guard uriComponents.count >= 2,
let providerForServiceName = servicesByName[uriComponents[0]],
let callHandler = providerForServiceName.handleMethod(uriComponents[1], callHandlerContext: context) else {
self.logger.notice("could not create handler", metadata: ["path": "\(requestHead.uri)"])
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/GRPC/Server.swift
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ extension Server {
}

fileprivate extension Server.Configuration {
var serviceProvidersByName: [String: CallHandlerProvider] {
var serviceProvidersByName: [Substring: CallHandlerProvider] {
return Dictionary(uniqueKeysWithValues: self.serviceProviders.map { ($0.serviceName, $0) })
}
}
Expand Down
12 changes: 6 additions & 6 deletions Sources/GRPCInteroperabilityTestModels/Generated/test.grpc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -382,11 +382,11 @@ public protocol Grpc_Testing_TestServiceProvider: CallHandlerProvider {
}

extension Grpc_Testing_TestServiceProvider {
public var serviceName: String { return "grpc.testing.TestService" }
public var serviceName: Substring { return "grpc.testing.TestService" }

/// Determines, calls and returns the appropriate request handler, depending on the request's method.
/// Returns nil for methods not handled by this service.
public func handleMethod(_ methodName: String, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
public func handleMethod(_ methodName: Substring, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
switch methodName {
case "EmptyCall":
return CallHandlerFactory.makeUnary(callHandlerContext: callHandlerContext) { context in
Expand Down Expand Up @@ -443,11 +443,11 @@ public protocol Grpc_Testing_UnimplementedServiceProvider: CallHandlerProvider {
}

extension Grpc_Testing_UnimplementedServiceProvider {
public var serviceName: String { return "grpc.testing.UnimplementedService" }
public var serviceName: Substring { return "grpc.testing.UnimplementedService" }

/// Determines, calls and returns the appropriate request handler, depending on the request's method.
/// Returns nil for methods not handled by this service.
public func handleMethod(_ methodName: String, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
public func handleMethod(_ methodName: Substring, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
switch methodName {
case "UnimplementedCall":
return CallHandlerFactory.makeUnary(callHandlerContext: callHandlerContext) { context in
Expand All @@ -468,11 +468,11 @@ public protocol Grpc_Testing_ReconnectServiceProvider: CallHandlerProvider {
}

extension Grpc_Testing_ReconnectServiceProvider {
public var serviceName: String { return "grpc.testing.ReconnectService" }
public var serviceName: Substring { return "grpc.testing.ReconnectService" }

/// Determines, calls and returns the appropriate request handler, depending on the request's method.
/// Returns nil for methods not handled by this service.
public func handleMethod(_ methodName: String, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
public func handleMethod(_ methodName: Substring, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
switch methodName {
case "Start":
return CallHandlerFactory.makeUnary(callHandlerContext: callHandlerContext) { context in
Expand Down
4 changes: 2 additions & 2 deletions Sources/protoc-gen-grpc-swift/Generator-Server.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ extension Generator {
println()
println("extension \(providerName) {")
indent()
println("\(access) var serviceName: String { return \"\(servicePath)\" }")
println("\(access) var serviceName: Substring { return \"\(servicePath)\" }")
println()
println("/// Determines, calls and returns the appropriate request handler, depending on the request's method.")
println("/// Returns nil for methods not handled by this service.")
println("\(access) func handleMethod(_ methodName: String, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {")
println("\(access) func handleMethod(_ methodName: Substring, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {")
indent()
println("switch methodName {")
for method in service.methods {
Expand Down
4 changes: 2 additions & 2 deletions Tests/GRPCTests/GRPCCustomPayloadTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ class GRPCCustomPayloadTests: GRPCTestCase {
// MARK: Custom Payload Service

fileprivate class CustomPayloadProvider: CallHandlerProvider {
var serviceName: String = "CustomPayload"
var serviceName: Substring = "CustomPayload"

fileprivate func reverseString(
request: StringPayload,
Expand Down Expand Up @@ -214,7 +214,7 @@ fileprivate class CustomPayloadProvider: CallHandlerProvider {
})
}

func handleMethod(_ methodName: String, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
func handleMethod(_ methodName: Substring, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
switch methodName {
case "Reverse":
return CallHandlerFactory.makeUnary(callHandlerContext: callHandlerContext) { context in
Expand Down
4 changes: 2 additions & 2 deletions dev/codegen-tests/01-echo/golden/echo.grpc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,11 @@ internal protocol Echo_EchoProvider: CallHandlerProvider {
}

extension Echo_EchoProvider {
internal var serviceName: String { return "echo.Echo" }
internal var serviceName: Substring { return "echo.Echo" }

/// Determines, calls and returns the appropriate request handler, depending on the request's method.
/// Returns nil for methods not handled by this service.
internal func handleMethod(_ methodName: String, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
internal func handleMethod(_ methodName: Substring, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
switch methodName {
case "Get":
return CallHandlerFactory.makeUnary(callHandlerContext: callHandlerContext) { context in
Expand Down
4 changes: 2 additions & 2 deletions dev/codegen-tests/02-multifile/golden/a.grpc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,11 @@ internal protocol A_ServiceAProvider: CallHandlerProvider {
}

extension A_ServiceAProvider {
internal var serviceName: String { return "a.ServiceA" }
internal var serviceName: Substring { return "a.ServiceA" }

/// Determines, calls and returns the appropriate request handler, depending on the request's method.
/// Returns nil for methods not handled by this service.
internal func handleMethod(_ methodName: String, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
internal func handleMethod(_ methodName: Substring, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
switch methodName {
case "CallServiceA":
return CallHandlerFactory.makeUnary(callHandlerContext: callHandlerContext) { context in
Expand Down
4 changes: 2 additions & 2 deletions dev/codegen-tests/02-multifile/golden/b.grpc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,11 @@ internal protocol B_ServiceBProvider: CallHandlerProvider {
}

extension B_ServiceBProvider {
internal var serviceName: String { return "b.ServiceB" }
internal var serviceName: Substring { return "b.ServiceB" }

/// Determines, calls and returns the appropriate request handler, depending on the request's method.
/// Returns nil for methods not handled by this service.
internal func handleMethod(_ methodName: String, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
internal func handleMethod(_ methodName: Substring, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
switch methodName {
case "CallServiceB":
return CallHandlerFactory.makeUnary(callHandlerContext: callHandlerContext) { context in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ internal protocol A_ServiceAProvider: CallHandlerProvider {
}

extension A_ServiceAProvider {
internal var serviceName: String { return "a.ServiceA" }
internal var serviceName: Substring { return "a.ServiceA" }

/// Determines, calls and returns the appropriate request handler, depending on the request's method.
/// Returns nil for methods not handled by this service.
internal func handleMethod(_ methodName: String, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
internal func handleMethod(_ methodName: Substring, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
switch methodName {
case "CallServiceA":
return CallHandlerFactory.makeUnary(callHandlerContext: callHandlerContext) { context in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,11 @@ internal protocol B_ServiceBProvider: CallHandlerProvider {
}

extension B_ServiceBProvider {
internal var serviceName: String { return "b.ServiceB" }
internal var serviceName: Substring { return "b.ServiceB" }

/// Determines, calls and returns the appropriate request handler, depending on the request's method.
/// Returns nil for methods not handled by this service.
internal func handleMethod(_ methodName: String, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
internal func handleMethod(_ methodName: Substring, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
switch methodName {
case "CallServiceB":
return CallHandlerFactory.makeUnary(callHandlerContext: callHandlerContext) { context in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,11 @@ internal protocol Codegentest_FooProvider: CallHandlerProvider {
}

extension Codegentest_FooProvider {
internal var serviceName: String { return "codegentest.Foo" }
internal var serviceName: Substring { return "codegentest.Foo" }

/// Determines, calls and returns the appropriate request handler, depending on the request's method.
/// Returns nil for methods not handled by this service.
internal func handleMethod(_ methodName: String, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
internal func handleMethod(_ methodName: Substring, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
switch methodName {
case "Get":
return CallHandlerFactory.makeUnary(callHandlerContext: callHandlerContext) { context in
Expand Down
4 changes: 2 additions & 2 deletions dev/codegen-tests/05-service-only/golden/test.grpc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,11 @@ internal protocol Codegentest_FooProvider: CallHandlerProvider {
}

extension Codegentest_FooProvider {
internal var serviceName: String { return "codegentest.Foo" }
internal var serviceName: Substring { return "codegentest.Foo" }

/// Determines, calls and returns the appropriate request handler, depending on the request's method.
/// Returns nil for methods not handled by this service.
internal func handleMethod(_ methodName: String, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
internal func handleMethod(_ methodName: Substring, callHandlerContext: CallHandlerContext) -> GRPCCallHandler? {
switch methodName {
case "Bar":
return CallHandlerFactory.makeUnary(callHandlerContext: callHandlerContext) { context in
Expand Down

0 comments on commit 6158fad

Please sign in to comment.