m_ticket_lifetime_hint is now a plain uint32_t without a default initializer. The constructor New_Session_Ticket_12(Handshake_IO&, Handshake_Hash&) calls io.send(*this) which will serialize the object and read m_ticket_lifetime_hint before it is set, causing undefined behavior. Initialize the member (e.g., uint32_t m_ticket_lifetime_hint{0};) and/or set it explicitly in the constructor.

policy().session_ticket_lifetime().count() is narrowed to uint32_t without bounds checking. If a policy returns a very large duration, this truncates and could produce an incorrect lifetime hint on the wire. Consider clamping to UINT32_MAX (and ideally rejecting negative values) before casting.

session_ticket_lifetime().count() is cast to uint32_t here as well. To avoid truncation/undefined semantics for very large policy durations, clamp to UINT32_MAX (and handle negative durations if possible) before narrowing.

This header now forward-declares Session_with_Handle/Session and returns std::vector<Session_with_Handle> by value (e.g. find(), find_some()). std::vector<T> requires T to be complete in translation units that construct/destroy the vector, so removing <botan/tls_session.h> is likely to break downstream builds. Consider including the header that defines Session_with_Handle (currently tls_session.h) or refactoring the API to avoid returning std::vector<Session_with_Handle> by value from this public header.

This header now forward-declares X509_Certificate but still exposes std::vector<X509_Certificate> by value in the public API (peer_cert_chain()). std::vector<T> generally requires T to be complete in translation units that construct/destroy it, so removing <botan/x509cert.h> is likely to break downstream builds. Consider keeping the include (or changing the API to avoid std::vector<X509_Certificate> by value).

It really stinks that <chrono> is so heavy. Ditching ::seconds (and friends) as strong types does reduce the expressiveness of the code. :( ... it's what it is, I guess.

I know, it's incredibly frustrating when the basic time unit types were so useful as interface types and then the committee decided to throw in so much stuff in to the header that they are no longer viable for that purpose.

IMO C++ is failing badly at both "You don't pay for what you don't use" and "Leave no room for a lower-level language" and the worst part is from all visible signs the committee thinks it's going great.

#JustUseModules is the answer, I guess...

The way I see it, You don't pay for what you don't use" only accounts for the final binary. And there it delivers in most cases. But compile times are a significant issue if you don't happen to have a 64 core beast in your basement.

The way I see it, You don't pay for what you don't use" only accounts for the final binary.

That's clearly their interpretation as well. Unfortunately that interpretation assumes developer wait time is free, which is not necessarily accurate.

Does it really hurt so much to pull in X509_DN ("pkix_types.h") here? In ::maybe_create this actually uses this lvalue to avoid a copy. Not that it is a particularly hot path in the code, but I'm wondering about the general rule-of-thumb for the trade-off between compile time and run time efficiency.

Similarly in Certificate_Authorities.

I guess it's a matter of slow cuts. Adding pkix_types.h slows down compilation of tls_messages.h by about 8% (.71s->.76s for GCC, 1.0s->1.07s for Clang) which is not so bad. But tls_messages.h is then included into 40 or 50 TUs. So making it even slightly more expensive balloons out.

Where exactly the line is drawn is certainly open for debate but broadly

• If something is included into many TUs it should be as light as possible. It doesn't matter if any single particular source file is slow. For example kuznyechik.o is one of the very slowest object files in the entire library to compile, due to fancy consteval tricks, and do we really need to be doing those, no - but it's just one file and not hurting anything else so who cares. Whereas eg concepts.h or loadstor.h being even 1ms faster to parse cuts 10s of seconds off the build. Also being common headers they are an ongoing tax; when another source file is added which includes them, the overall compilation cost of these headers grows further.

• If the performance doesn't matter much (for example, if something could cost twice as much and nobody would ever notice) then might as well prefer saving compilation time, since compilation time is a scarce resource of uncertain availability [1] and that resource is shared across the entire codebase.

[1]: In that there is no fixed hard budget but slower compilation time hurts developer velocity and likely as not hurts adoption if it's bad enough to notice.

In this specific case, I'd say: many TUs are affected, it's not really performance critical (only happens during the handshake and the handshake is anyway expensive), so prefer saving compilation time. I guess if we had profiles for the TLS handshake performance [which would be a useful benchmark to have!!] and the extra copy was showing up somehow, that would be worth working on.

As an example going the other way, headers like ct_utils.h and simd_4x32.h are somewhat expensive, about .25-.35 seconds to compile, and used everywhere. But these are run in hot loops that are written expecting values like CT::Mask and SIMD_4x32 will be perfectly inlined, it's difficult to reduce the compilation time meaningfully without drastically impacting performance.

Thanks for the guidelines. I concur in general terms. Though, still I feel sad to introduce runtime efficiencies like that. But from a gut feeling I also don't think this particular copy would produce measurable runtime slowdowns. On that note, we're starting to use the TLS stack in a system that might actually experience some significant load. Therefore, benchmarks for TLS might come up on my TODO list eventually...

Anyway: with all the header patrol work you've put in, I think it would be really cool to have an automated compile benchmark in CI to catch regressions early and weigh the pros and cons of new API designs early on. (For instance the new loadstor.h is convenient but I do question whether it was really necessary to push the limits that far. 😶)

I think it would be really cool to have an automated compile benchmark in CI to catch regressions early and weigh the pros and cons of new API designs early on.

Yes I have something specifically like this in mind to include in the nightly run (and can be invoked on demand if a PR seems risky in this way) - Clang's -ftime-trace dumps a lot of useful information.