cache NIOSSLContext (saves 27k allocs per conn)

[weissi]

Motivation:

At the moment, AHC assumes that creating a NIOSSLContext is both cheap
and doesn't block.

Neither of these two assumptions are true.

To create a NIOSSLContext, BoringSSL will have to read a lot of
certificates in the trust store (on disk) which require a lot of ASN1
parsing and much much more.

On my Ubuntu test machine, creating one NIOSSLContext is about 27,000
allocations!!! To make it worse, AHC allocates a fresh NIOSSLContext
for every single connection, whether HTTP or HTTPS. Yes, correct.

Modification:

• Cache NIOSSLContexts per TLSConfiguration in a LRU cache
• Don't get an NIOSSLContext for HTTP (plain text) connections

Result:

New connections should be much faster in general assuming that you're
not using a different TLSConfiguration for every connection.

[weissi]

before

49,640 allocations for 100 plain-text HTTP requests over 1 connection

after

22,158 allocations for 100 plain-text HTTP requests over 1 connection

[weissi]

note: make this private because nobody accesses it from the outside and if not private, then there are places where we have both a key.tlsConfiguration and a configuration.tlsConfiguration which aren't necessarily the same (eg. when doing https:// over a http:// proxy).

[fabianfett]

Is there a reason, why we don't use this pattern?

Suggested change

	var isNIOTS: Bool {
	#if canImport(Network)
	if #available(OSX 10.14, iOS 12.0, tvOS 12.0, watchOS 6.0, *) {
	return self.underlyingBootstrap is NIOTSConnectionBootstrap
	} else {
	return false
	}
	#else
	return false
	#endif
	}
	var isNIOTS: Bool {
	#if canImport(Network)
	if #available(OSX 10.14, iOS 12.0, tvOS 12.0, watchOS 6.0, *) {
	return self.underlyingBootstrap is NIOTSConnectionBootstrap
	}
	#endif
	return false
	}

[weissi]

I reduced the amount of code we need here but happy to add another } else { return false }

[fabianfett]

Why do we pass the SSLContextCache down here? We already have a sslContext (a Future) created before. IMHO it would make more sense to pass that down.

[weissi]

That is an excellent question, I have no idea. I'll look into this, maybe we can just inject the SSLContext into that method.

[weissi]

Because we now need the actual context. We're kicking off the connection creation and an SSLContext creation in parallel and this is the point where we need both.

[fabianfett]

This is hit with in a test that doesn't require https: testUploadStreamingBackpressure

[weissi]

great catch! Thanks you

[Lukasa]

Cool, this LGTM.

[fabianfett]

LGTM. 🎉