Make HPACKHeaders inlinable. #269
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Motivation: HPACKHeaders is a trivial data structure (a wrapper around Array) that is present in the public API of all NIO HTTP/2 programs. This makes it a tempting target for adding inlinable calls, primarily as a way to drive down ARC. The fact that strings frequently travel across the API boundary means that otherwise innocuous use-cases can trigger unexpected ARC traffic. Making as much of the implementation inlinable as possible reduces a lot of this overhead and opens up the possibility for better performance in a number of parts of the code, including in the core HTTP/2 protocol engine, which operates extensively on this data type. Modifications: - Make basically everything inlinable. Result: Our benchmarks are about 0.7% faster.
Lukasa
force-pushed
the
cb-inlinable-hpack
branch
from
efa07d5 to
6d48cdc
Compare
PeterAdams-A
approved these changes
Dec 4, 2020
| @inline(never) | ||
| private func _compareCaseInsensitiveASCIIBytesSlowPath(to other: String.UTF8View) -> Bool { | ||
| func _compareCaseInsensitiveASCIIBytesSlowPath(to other: String.UTF8View) -> Bool { |
There was a problem hiding this comment.
This looks odd - inlineable but never?
There was a problem hiding this comment.
Yup. This is a logically sensible stanza. @inlinable means the definition is available to the compiler when compiling the module using the code. This allows, for example, specialization. But the method may not be inlined. In this case the primary use is to let the compiler see through the call and observe that we never write to any memory here.
| @@ -461,7 +502,8 @@ extension String.UTF8View { | |||
| /// | |||
| /// - Parameter bytes: The string constant in the form of a collection of `UInt8` | |||
| /// - Returns: Whether the collection contains **EXACTLY** this array or no, but by ignoring case. | |||
| fileprivate func compareCaseInsensitiveASCIIBytes(to other: String.UTF8View) -> Bool { | |||
| @inlinable | |||
There was a problem hiding this comment.
I've not read the rules but I don't see any logical reason why something shouldn't be both inlineable and fileprivate.
There was a problem hiding this comment.
Totally reasonable except that the rule is it has to be internal or higher.
glbrntt
approved these changes
Dec 8, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
HPACKHeaders is a trivial data structure (a wrapper around Array) that
is present in the public API of all NIO HTTP/2 programs. This makes it a
tempting target for adding inlinable calls, primarily as a way to drive
down ARC. The fact that strings frequently travel across the API
boundary means that otherwise innocuous use-cases can trigger unexpected
ARC traffic.
Making as much of the implementation inlinable as possible reduces a lot
of this overhead and opens up the possibility for better performance in
a number of parts of the code, including in the core HTTP/2 protocol
engine, which operates extensively on this data type.
Modifications:
Result:
Our benchmarks are about 0.7% faster.