Skip to content

Commit 326f7f9

Browse files
LukasaLukasa
authored
Simplify the canonicalForm subscript further. (#310)
Motivation: The canonicalForm subscript was written for clarity, but it didn't do a good job of avoiding performance costs. Unfortunately, this function can be a bit costly, for two reasons. Firstly, it used a bunch of eager collection operations, such that it would allocate intermediate arrays. Secondly, it computed on the String, rather than the UTF8View backing that string, in order to perform those collection operations. The result was that it spent a bit too much time messing about with the headers than it needed to. Modifications: - Define a lazy UTF8 view splitting sequence and use it. - Change to compute on the UTF8View. Result: Better performance in the canonicalForm subscript.
1 parent 2624ccc commit 326f7f9

File tree

6 files changed

+103
-34
lines changed

6 files changed

+103
-34
lines changed

‎Sources/NIOHPACK/HPACKHeader.swift

Lines changed: 83 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -260,15 +260,25 @@ public struct HPACKHeaders: ExpressibleByDictionaryLiteral {
260260
return result
261261
}
262262

263-
return result.flatMap {
264-
$0.split(separator: ",")
265-
}.lazy.map {
266-
$0._trimWhitespace()
267-
}.filter { // `split(separator:)` drops empty strings, we should too.
268-
!$0.isEmpty
269-
}.map {
270-
String($0)
263+
// We slightly overcommit here to try to reduce the amount of resizing we do.
264+
var trimmedResults: [String] = []
265+
trimmedResults.reserveCapacity(result.count * 4)
266+
267+
// This loop operates entirely on the UTF-8 views. This vastly reduces the cost of this slicing and dicing.
268+
for field in result {
269+
for entry in field.utf8._lazySplit(separator: UInt8(ascii: ",")) {
270+
let trimmed = entry._trimWhitespace()
271+
if trimmed.isEmpty {
272+
continue
273+
}
274+
275+
// This constructor pair kinda sucks, but you can't create a String from a slice of UTF8View as
276+
// cheaply as you can with a Substring, so we go through that initializer instead.
277+
trimmedResults.append(String(Substring(trimmed)))
278+
}
271279
}
280+
281+
return trimmedResults
272282
}
273283

274284
/// Special internal function for use by tests.
@@ -488,17 +498,76 @@ internal extension UTF8.CodeUnit {
488498
}
489499
}
490500

491-
extension Substring {
501+
extension Substring.UTF8View {
492502
@inlinable
493-
func _trimWhitespace() -> Substring {
494-
guard let firstNonWhitespace = self.utf8.firstIndex(where: { !$0.isASCIIWhitespace }) else {
503+
func _trimWhitespace() -> Substring.UTF8View {
504+
guard let firstNonWhitespace = self.firstIndex(where: { !$0.isASCIIWhitespace }) else {
495505
// The whole substring is ASCII whitespace.
496-
return Substring()
506+
return Substring().utf8
497507
}
498508

499509
// There must be at least one non-ascii whitespace character, so banging here is safe.
500-
let lastNonWhitespace = self.utf8.lastIndex(where: { !$0.isASCIIWhitespace })!
501-
return Substring(self.utf8[firstNonWhitespace...lastNonWhitespace])
510+
let lastNonWhitespace = self.lastIndex(where: { !$0.isASCIIWhitespace })!
511+
return self[firstNonWhitespace...lastNonWhitespace]
512+
}
513+
}
514+
515+
extension String.UTF8View {
516+
@inlinable
517+
func _lazySplit(separator: UTF8.CodeUnit) -> LazyUTF8ViewSplitSequence {
518+
return LazyUTF8ViewSplitSequence(self, separator: separator)
519+
}
520+
521+
@usableFromInline
522+
struct LazyUTF8ViewSplitSequence: Sequence {
523+
@usableFromInline typealias Element = Substring.UTF8View
524+
525+
@usableFromInline var _baseView: String.UTF8View
526+
@usableFromInline var _separator: UTF8.CodeUnit
527+
528+
@inlinable
529+
init(_ baseView: String.UTF8View, separator: UTF8.CodeUnit) {
530+
self._baseView = baseView
531+
self._separator = separator
532+
}
533+
534+
@inlinable
535+
func makeIterator() -> Iterator {
536+
return Iterator(self)
537+
}
538+
539+
@usableFromInline
540+
struct Iterator: IteratorProtocol {
541+
@usableFromInline var _base: LazyUTF8ViewSplitSequence
542+
@usableFromInline var _lastSplitIndex: Substring.UTF8View.Index
543+
544+
@inlinable
545+
init(_ base: LazyUTF8ViewSplitSequence) {
546+
self._base = base
547+
self._lastSplitIndex = base._baseView.startIndex
548+
}
549+
550+
@inlinable
551+
mutating func next() -> Substring.UTF8View? {
552+
let endIndex = self._base._baseView.endIndex
553+
554+
guard self._lastSplitIndex != endIndex else {
555+
return nil
556+
}
557+
558+
let restSlice = self._base._baseView[self._lastSplitIndex...]
559+
560+
if let nextSplitIndex = restSlice.firstIndex(of: self._base._separator) {
561+
// The separator is present. We want to drop the separator, so we need to advance the index past this point.
562+
self._lastSplitIndex = self._base._baseView.index(after: nextSplitIndex)
563+
return restSlice[..<nextSplitIndex]
564+
} else {
565+
// The separator isn't present, so we want the entire rest of the slice.
566+
self._lastSplitIndex = self._base._baseView.endIndex
567+
return restSlice
568+
}
569+
}
570+
}
502571
}
503572
}
504573

‎dev/update-alloc-limits-to-last-completed-ci-build

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ url_prefix=${1-"https://ci.swiftserver.group/job/swift-nio-http2-2-"}
2222
target_repo=${2-"$here/.."}
2323
tmpdir=$(mktemp -d /tmp/.last-build_XXXXXX)
2424

25-
for f in swift51 swift52 swift53 swift54 swift55 nightly; do
25+
for f in swift52 swift53 swift54 swift55 nightly; do
2626
echo "$f"
2727
url="$url_prefix$f-prb/lastCompletedBuild/consoleFull"
2828
stripped=${f#"swift"}

‎docker/docker-compose.1604.52.yaml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,13 @@ services:
2727
environment:
2828
- MAX_ALLOCS_ALLOWED_1k_requests_interleaved=51150
2929
- MAX_ALLOCS_ALLOWED_1k_requests_noninterleaved=50100
30-
- MAX_ALLOCS_ALLOWED_client_server_h1_request_response=325000
31-
- MAX_ALLOCS_ALLOWED_client_server_request_response=289000
30+
- MAX_ALLOCS_ALLOWED_client_server_h1_request_response=324900
31+
- MAX_ALLOCS_ALLOWED_client_server_request_response=288900
3232
- MAX_ALLOCS_ALLOWED_create_client_stream_channel=48050
33-
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form=700050
34-
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace=700050
35-
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_long_string=800050
36-
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_short_string=700050
33+
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form=200050
34+
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace=200050
35+
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_long_string=300050
36+
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_short_string=200050
3737
- MAX_ALLOCS_ALLOWED_hpack_decoding=5050
3838
- MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent=353200
3939
- SANITIZER_ARG=--sanitize=thread

‎docker/docker-compose.1804.53.yaml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ services:
3030
- MAX_ALLOCS_ALLOWED_client_server_h1_request_response=324000
3131
- MAX_ALLOCS_ALLOWED_client_server_request_response=288000
3232
- MAX_ALLOCS_ALLOWED_create_client_stream_channel=48050
33-
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form=700050
34-
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace=700050
35-
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_long_string=800050
36-
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_short_string=700050
33+
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form=200050
34+
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace=200050
35+
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_long_string=300050
36+
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_short_string=200050
3737
- MAX_ALLOCS_ALLOWED_hpack_decoding=5050
3838
- MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent=333200
3939

‎docker/docker-compose.2004.54.yaml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ services:
3131
- MAX_ALLOCS_ALLOWED_client_server_h1_request_response=324000
3232
- MAX_ALLOCS_ALLOWED_client_server_request_response=288000
3333
- MAX_ALLOCS_ALLOWED_create_client_stream_channel=48050
34-
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form=700050
35-
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace=700050
36-
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_long_string=800050
37-
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_short_string=700050
34+
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form=200050
35+
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace=200050
36+
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_long_string=300050
37+
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_short_string=200050
3838
- MAX_ALLOCS_ALLOWED_hpack_decoding=5050
3939
- MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent=333200
4040

‎docker/docker-compose.2004.55.yaml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,12 @@ services:
3030
- MAX_ALLOCS_ALLOWED_client_server_h1_request_response=324000
3131
- MAX_ALLOCS_ALLOWED_client_server_request_response=288000
3232
- MAX_ALLOCS_ALLOWED_create_client_stream_channel=48050
33-
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form=700050
34-
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace=700050
35-
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_long_string=800050
36-
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_short_string=700050
33+
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form=200050
34+
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace=200050
35+
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_long_string=300050
36+
- MAX_ALLOCS_ALLOWED_get_100000_headers_canonical_form_trimming_whitespace_from_short_string=200050
3737
- MAX_ALLOCS_ALLOWED_hpack_decoding=5050
38-
- MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent=333200
38+
- MAX_ALLOCS_ALLOWED_stream_teardown_100_concurrent=333150
3939

4040
shell:
4141
image: swift-nio-http2:20.04-5.5

0 commit comments

Comments
 (0)