pem: avoid some allocations

[djc]

+57 lines so it's not free, but IMO the complexity isn't much worse, by virtue of using more descriptive types/functions and pushing complexity to the edges.

What do you think?

@cpu want to run this through your test setup?

[cpu]

@cpu want to run this through your test setup?

Haven't reviewed the contents of the diff yet, but speaking strictly about the effects it has in heaptrack w/ curl: very nice!

The baseline I shared the other day was rustls-ffi 0.15 and curl 8.14.1:

• ~2k allocations total.
  • 781 from rustls_pki_types::pem::read

Here's the same test but rustls-ffi built with rustls-pki-types @ e88375b as a cargo patch:

• down to ~1,200 allocations total
  • 319 from rustls_pki_types::pem_read

A quick back-of-the-envelope check (cat /etc/ssl/certs/ca-certificates.crt | grep -c "BEGIN") shows 319 certs in my /etc/ssl/certs bundle so that seems spot on for the count of pem::read() allocations 👍 We winnow that down to 286 TrustAnchor::to_owned() allocations in both cases.

[djc]

I'm a little enticed to build a parser type that can hold on to the temporary buffers across PEM files for use in rustls-native-certs, but maybe that's overkill?

[ctz]

nice!

[ctz]

I'm a little enticed to build a parser type that can hold on to the temporary buffers across PEM files for use in rustls-native-certs, but maybe that's overkill?

I think for rustls-native-certs with its current API, the best we could do is one allocation per certificate. So another option would be making the base64 decoder in this crate operate in-place, meaning that the temporary buffer for PEM decoding is the one returned in each CertificateDer. Some different trade-offs there though.

[djc]

I think for rustls-native-certs with its current API, the best we could do is one allocation per certificate. So another option would be making the base64 decoder in this crate operate in-place, meaning that the temporary buffer for PEM decoding is the one returned in each CertificateDer. Some different trade-offs there though.

What trade-offs are you thinking about?

[cpu]

Really nice 🤩

[djc]

I'm a little enticed to build a parser type that can hold on to the temporary buffers across PEM files for use in rustls-native-certs, but maybe that's overkill?

I think for rustls-native-certs with its current API, the best we could do is one allocation per certificate. So another option would be making the base64 decoder in this crate operate in-place, meaning that the temporary buffer for PEM decoding is the one returned in each CertificateDer. Some different trade-offs there though.

Thinking about this more, we're already doing ~1 allocation per certificate in the scenario under test. Opportunities for further improvement:

• Don't allocate for reading PEM objects from SliceIter -- these could reference the original slice instead (requires a semver-breaking change for rustls-pki-types)
• Read TrustAnchor directly from PEM -- parsing TrustAnchor from certificate DER now lives in webpki which is more convenient from a DER parsing angle but otherwise requires a bit of indirection
• When reading PEM objects from multiple files, could reuse parse buffers (both the line buffer and the b64 buf) across files

Feels like none of these are as low-hanging as the stuff I did here, so I'm not convinced it's worth chasing them down now.