From 25caf08e75b71ed5b87408c7467ee60e78073a91 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Fri, 7 Aug 2020 10:44:14 +0100 Subject: [PATCH 1/2] Improve performance of routing inbound RPCs. 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. --- .../GRPC/GRPCServerRequestRoutingHandler.swift | 17 ++++++++--------- Sources/GRPC/Server.swift | 2 +- .../Generator-Server.swift | 4 ++-- Tests/GRPCTests/GRPCCustomPayloadTests.swift | 4 ++-- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/Sources/GRPC/GRPCServerRequestRoutingHandler.swift b/Sources/GRPC/GRPCServerRequestRoutingHandler.swift index ce9d3f757..95839ba4d 100644 --- a/Sources/GRPC/GRPCServerRequestRoutingHandler.swift +++ b/Sources/GRPC/GRPCServerRequestRoutingHandler.swift @@ -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 @@ -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 @@ -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? @@ -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 @@ -219,7 +218,7 @@ 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, @@ -227,9 +226,9 @@ extension GRPCServerRequestRoutingHandler: ChannelInboundHandler, RemovableChann 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 } diff --git a/Sources/GRPC/Server.swift b/Sources/GRPC/Server.swift index c12ae03cc..23f18d83d 100644 --- a/Sources/GRPC/Server.swift +++ b/Sources/GRPC/Server.swift @@ -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) }) } } diff --git a/Sources/protoc-gen-grpc-swift/Generator-Server.swift b/Sources/protoc-gen-grpc-swift/Generator-Server.swift index 4bdce5f81..a22e5c0fb 100644 --- a/Sources/protoc-gen-grpc-swift/Generator-Server.swift +++ b/Sources/protoc-gen-grpc-swift/Generator-Server.swift @@ -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 { diff --git a/Tests/GRPCTests/GRPCCustomPayloadTests.swift b/Tests/GRPCTests/GRPCCustomPayloadTests.swift index 3e72baff7..b71b06233 100644 --- a/Tests/GRPCTests/GRPCCustomPayloadTests.swift +++ b/Tests/GRPCTests/GRPCCustomPayloadTests.swift @@ -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, @@ -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 From 2d45ccf662d8933e946770eaf27480a73d751785 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Fri, 7 Aug 2020 11:56:14 +0100 Subject: [PATCH 2/2] Regenerate protos --- Sources/Examples/Echo/Model/echo.grpc.swift | 4 ++-- .../Examples/HelloWorld/Model/helloworld.grpc.swift | 4 ++-- .../Examples/RouteGuide/Model/route_guide.grpc.swift | 4 ++-- .../Generated/test.grpc.swift | 12 ++++++------ dev/codegen-tests/01-echo/golden/echo.grpc.swift | 4 ++-- dev/codegen-tests/02-multifile/golden/a.grpc.swift | 4 ++-- dev/codegen-tests/02-multifile/golden/b.grpc.swift | 4 ++-- .../03-multifile-with-module-map/golden/a.grpc.swift | 4 ++-- .../03-multifile-with-module-map/golden/b.grpc.swift | 4 ++-- .../golden/service.grpc.swift | 4 ++-- .../05-service-only/golden/test.grpc.swift | 4 ++-- 11 files changed, 26 insertions(+), 26 deletions(-) diff --git a/Sources/Examples/Echo/Model/echo.grpc.swift b/Sources/Examples/Echo/Model/echo.grpc.swift index 175e106aa..2f26a39e5 100644 --- a/Sources/Examples/Echo/Model/echo.grpc.swift +++ b/Sources/Examples/Echo/Model/echo.grpc.swift @@ -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 diff --git a/Sources/Examples/HelloWorld/Model/helloworld.grpc.swift b/Sources/Examples/HelloWorld/Model/helloworld.grpc.swift index 583994682..8783c7803 100644 --- a/Sources/Examples/HelloWorld/Model/helloworld.grpc.swift +++ b/Sources/Examples/HelloWorld/Model/helloworld.grpc.swift @@ -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 diff --git a/Sources/Examples/RouteGuide/Model/route_guide.grpc.swift b/Sources/Examples/RouteGuide/Model/route_guide.grpc.swift index 00dbd8b9c..789326e82 100644 --- a/Sources/Examples/RouteGuide/Model/route_guide.grpc.swift +++ b/Sources/Examples/RouteGuide/Model/route_guide.grpc.swift @@ -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 diff --git a/Sources/GRPCInteroperabilityTestModels/Generated/test.grpc.swift b/Sources/GRPCInteroperabilityTestModels/Generated/test.grpc.swift index 7cd06ea0d..1aab97fcb 100644 --- a/Sources/GRPCInteroperabilityTestModels/Generated/test.grpc.swift +++ b/Sources/GRPCInteroperabilityTestModels/Generated/test.grpc.swift @@ -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 @@ -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 @@ -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 diff --git a/dev/codegen-tests/01-echo/golden/echo.grpc.swift b/dev/codegen-tests/01-echo/golden/echo.grpc.swift index d53399c09..4df7ef463 100644 --- a/dev/codegen-tests/01-echo/golden/echo.grpc.swift +++ b/dev/codegen-tests/01-echo/golden/echo.grpc.swift @@ -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 diff --git a/dev/codegen-tests/02-multifile/golden/a.grpc.swift b/dev/codegen-tests/02-multifile/golden/a.grpc.swift index 6adaa3773..6afe20676 100644 --- a/dev/codegen-tests/02-multifile/golden/a.grpc.swift +++ b/dev/codegen-tests/02-multifile/golden/a.grpc.swift @@ -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 diff --git a/dev/codegen-tests/02-multifile/golden/b.grpc.swift b/dev/codegen-tests/02-multifile/golden/b.grpc.swift index 735c433e3..8f9778cd7 100644 --- a/dev/codegen-tests/02-multifile/golden/b.grpc.swift +++ b/dev/codegen-tests/02-multifile/golden/b.grpc.swift @@ -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 diff --git a/dev/codegen-tests/03-multifile-with-module-map/golden/a.grpc.swift b/dev/codegen-tests/03-multifile-with-module-map/golden/a.grpc.swift index 7fec409f3..b53e27ba9 100644 --- a/dev/codegen-tests/03-multifile-with-module-map/golden/a.grpc.swift +++ b/dev/codegen-tests/03-multifile-with-module-map/golden/a.grpc.swift @@ -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 diff --git a/dev/codegen-tests/03-multifile-with-module-map/golden/b.grpc.swift b/dev/codegen-tests/03-multifile-with-module-map/golden/b.grpc.swift index 735c433e3..8f9778cd7 100644 --- a/dev/codegen-tests/03-multifile-with-module-map/golden/b.grpc.swift +++ b/dev/codegen-tests/03-multifile-with-module-map/golden/b.grpc.swift @@ -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 diff --git a/dev/codegen-tests/04-service-with-message-import/golden/service.grpc.swift b/dev/codegen-tests/04-service-with-message-import/golden/service.grpc.swift index 9d1466a3d..131c6dfa9 100644 --- a/dev/codegen-tests/04-service-with-message-import/golden/service.grpc.swift +++ b/dev/codegen-tests/04-service-with-message-import/golden/service.grpc.swift @@ -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 diff --git a/dev/codegen-tests/05-service-only/golden/test.grpc.swift b/dev/codegen-tests/05-service-only/golden/test.grpc.swift index db9f1191a..702f3de03 100644 --- a/dev/codegen-tests/05-service-only/golden/test.grpc.swift +++ b/dev/codegen-tests/05-service-only/golden/test.grpc.swift @@ -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