hide SSL implementation headers behind interfaces

[MartinNowak]

• turn SSLStream and SSLContext into interfaces
• move OpenSSL based implementation to vibe.stream.openssl

[MartinNowak]

Please review carefully, I just did this very mechanically.
It would probably make sense to turn SSLContext into a class instead of an interface as some methods should apply to any SSL implementation. Then those function could be made final methods.

This time I do have numbers. The raw compile time as measured here went down from 0.8s to 0.4s. Number of imported files was reduced from 202 to 152, LOC went from 222.5K to 192.7K and code size dropped from 8M to 6.8M.
It's a little surprising to see that the compile time reduced overproportional.

[etcimon]

It's a little surprising to see that the compile time reduced overproportional.

So, this means there could be a use to a pragma-based compile-time profiler? :-P

Best I could find is this: http://forum.dlang.org/thread/[email protected]

[MartinNowak]

Updated graph (http://jsfiddle.net/DJqjD/), the next victim will be dub :).

There is probably still some potential to reduce phobos/druntime imports.

[MartinNowak]

So, this means there could be a use to a pragma-based compile-time profiler? :-P

From the old and the new perf histogram one can see, that the compiler has to do a huge amount of symbol lookups for openssl.
Even when a module is only imported the compiler will the first semantic pass on it.
It might be interesting to analyse this a bit more. Maybe there are a lot of cross-module dependencies in the openssl headers or it's all those ExternC templates that slow down the compiler.

[s-ludwig]

The const seems to be accidentally dropped here.

[s-ludwig]

Apart from the missing const, everything looks fine.

Looks like an impressive compile time reduction for one small component. But judging be the monster that the OpenSSL API is, its actually not surprising ;)

BTW, Vladimir also made this useful little tool that would split up the build time into its components (that he also demonstrated on DConf): DBuildStat - that gave some interesting insights in an earlier attempt to reduce compile times.

[s-ludwig]

PS: I wanted to integrate some simple profiling into DUB that could be enabled with a command line switch, but didn't get around to it yet. That would probably help a lot to quickly identify where the time is wasted.

[MartinNowak]

Updated.

It would probably make sense to turn SSLContext into a class instead of an interface as some methods should apply to any SSL implementation.

So we could move the SSLContextKind, SSLPeerValidationCallback and SSLPeerValidationMode fields into an abstract class in vibe.stream.ssl. I don't think it's worth it though and it can always be done when there actually is another SSL implementation.

[MartinNowak]

To make another case for interfaces, it should now be possible to implement a SSL replacement or a different event driver in another library. Might be a good starting point to work on a native epoll or kqueue implementation.

[s-ludwig]

So we could move the SSLContextKind, SSLPeerValidationCallback and SSLPeerValidationMode fields into an abstract class in vibe.stream.ssl. I don't think it's worth it though and it can always be done when there actually is another SSL implementation.

I think so, too. They are used relatively rarely and are just two lines, so it doesn't relly buy much in terms of either performance or saving code.

To make another case for interfaces, it should now be possible to implement a SSL replacement or a different event driver in another library. Might be a good starting point to work on a native epoll or kqueue implementation.

Event divers are separate, they have always been full interfaces. But it's definitely good to have that possibility opened up for SSL streams, too. Now the only thing missing would be to have a registration facility for alternative implementations, so that createSSL(Stream/Context) creates the external implementation instead of the OpenSSL one.

[MartinNowak]

Would be a good selling point to have GNUTLS and PolarSSL plugins.

version (VibeCustomSSL) {
    interface SSLFactory {
        SSLContext createSSLContext(SSLContextKind kind, SSLVersion ver = SSLVersion.any);
        SSLStream createSSLStream(Stream underlying, SSLContext ctx, SSLStreamState state, string peer_name = null, NetworkAddress peer_address = NetworkAddress.init);
    }
    void setSSLFactory(SSLFactory factory) { s_sslFactory = factory; }
}

SSLStream createSSLStream(Stream underlying, SSLContext ctx, SSLStreamState state, string peer_name = null, NetworkAddress peer_address = NetworkAddress.init)
{
    version (VibeNoSSL) assert(false, "No SSL support compiled in (VibeNoSSL)");
    else version (OpenSSL) {
        import vibe.stream.openssl;
        return new OpenSSLStream(DEPRECATION_HACK.init, underlying, cast(OpenSSLContext)ctx,
                                 state, peer_name, peer_address);
    } else version (VibeCustomSSL) {
        enforce(s_sslFactory, "No SSL Factory provided.");
        return s_sslFactory.createSSLStream(underlying, ctx, state, peer_name, peer_address);
    }
}

Or plain function pointers.

version (VibeCustomSSL)
    void setCreateSSLStream(typeof(&createSSLStream) func) { s_createSSLStream = func; }

SSLStream createSSLStream(Stream underlying, SSLContext ctx, SSLStreamState state, string peer_name = null, NetworkAddress peer_address = NetworkAddress.init)
{
    // ...
    } else version (VibeCustomSSL)
        return s_createSSLStream(/*Args*/);
}

[s-ludwig]

Wait a minute... that DEPRECATION_HACK can actually be dropped now. It was only necessary for a non-breaking switch to interfaces. So VibeCustomSSL could also be avoided and the OpenSSLStream would just behave like any other implementation. Also, I see no reason against adding a createStream method to SSLContext, so that the registration would boil down to just a single SSLContext function():

void setSSLContextFactory(SSLContext function(SSLContextKind, SSLVersion) factory)
    { s_factory = factory }

void createSSLContext(SSLContextKind kind, SSLVersion ver)
    { return s_factory(kind, var); }

void createSSLStream(SSLStream underlying, SSLContext ctx, ...)
    { return ctx.createStream(underlying, ...);

[etcimon]

To make another case for interfaces, it should now be possible to implement a SSL replacement or a different event driver in another library. Might be a good starting point to work on a native epoll or kqueue implementation.

The sanest SSL stream would be Botan, I'll be using the algorithm factory for the TLS engine I'm working on but it does have a decent TLS engine already out of the box
https://github.com/randombit/botan/blob/net.randombit.botan/src/lib/tls/tls_server.h
https://github.com/randombit/botan/blob/net.randombit.botan/src/lib/tls/tls_client.h

Also, it has been shown to statically compile with an amalgamation script used in Titanium https://github.com/ellipticbit/titanium

It looks like it's a hot subject so I'm going to re-open my work on native events right now see if I can move it into a driver this week.

[MartinNowak]

It looks like it's a hot subject so I'm going to re-open my work on native events right now see if I can move it into a driver this week.

What are you working on, epoll, kqueue or something else?

[etcimon]

What are you working on, epoll, kqueue or something else?

I'm working on epoll and iocp at first. I put it on hold before finishing and making a pull request because I developed it too fast (10 days) and I wasn't sure if I would change my mind on certain things.

https://github.com/etcimon/vibe.d/tree/native-events/source/vibe/core/events