Add a fast path for the DefaultCertificateSelector #318
Conversation
In cases where we only have a single certificate to choose from we will anyways pick that certificate, regardless of whether the certificate is supported by the client or is expired.
|
Found while reading and profiling the whole "get certificate" path. The "supports certificate" function is actually rather complex, and if we anyways will return the certificate there is no point in calling it (or the expiration check) here. |
| if len(choices) == 0 { | ||
| return Certificate{}, fmt.Errorf("no certificates available") | ||
| } | ||
|
|
||
| // Slow path: There are choices, so we need to check each of them. | ||
| now := time.Now() |
There was a problem hiding this comment.
I didn't check, but: There might be a good opportunity to split the slow path out into a separate function, so that the fast path can be inlined.
There was a problem hiding this comment.
Should be doable. Lemme catch up on this next Caddy beta release and then I/we can explore that as a follow-up PR. I'd be curious if the inlining actually happens and if so does the performance improve?
There was a problem hiding this comment.
Thanks for this optimization, makes sense!
At first I read the comment, and was like, "No, wait, we shouldn't serve expired or incompatible certs." But then I realized that we already do this below (best := choices[0]) and remembered why... it's generally best practice to serve any matching certificate you have in response to a ClientHello because it can help with troubleshooting. Like if you serve an expired cert, you can instantly know that something about its renewal failed (for a long time!)... or if it's an incompatible cert, you can verify that everything on the server is correct and that the client program just needs to get over itself and support it 😅
Anyway, thanks again for this.
In cases where we only have a single certificate to choose from we will anyways pick that certificate, regardless of whether the certificate is supported by the client or is expired.