Re: [openssl-dev] Multithreading performance patches

[Andrey Kulikov]

Hi!

> The reason is that so far it was
> reserved for occasion like this, i.e. when somebody requests it and puts
> effort into testing it in real-life application:-) Can we do that?

I did it.
Added to crypto/cryptlib.c:

#  ifndef OPENSSL_DISABLE_ATOMIC_ADD_INTERNAL
int OPENSSL_atomic_add(int *, int, int, const char *, int);
#  endif

and insert at the end of void OPENSSL_cpuid_setup(void)

#  ifndef OPENSSL_DISABLE_ATOMIC_ADD_INTERNAL
CRYPTO_set_add_lock_callback(OPENSSL_atomic_add);
#  endif

So one could disable this feature by defining
OPENSSL_DISABLE_ATOMIC_ADD_INTERNAL macro at compile time.
Tested on Linux x86 and x64 during last 4 years on different high-load
SSL-servers.
Tested on Linux x86 and x64, Windows x86 and x64 in different SSL-client
applications.

No issues observed.

Cheers,
Andrey.

On 12 October 2012 at 18:19, Andy Polyakov  wrote:

> > I'm working on fast multithreaded TLS server for node.js, and I've
> > encountered few problems with openssl threading support which I would
> > like to address with attached patches.
> >
> > Please ask me if you have any questions or comments.
>
> As for 1st patch. Note that the section in question is guarded by 'if
> (add_lock_callback)'. add_lock_callback can be set by application and
> perform operation to its liking. In addition, on platforms with
> OPENSSL_CPUID_OBJ defined there is OPENSSL_atomic_add that can be used
> as callback, so that all you need to do then is
> CRYPTO_set_add_lock_callback(OPENSSL_atomic_add) in application code.
> Well, you'll surely get into trouble compiling this line, because
> OPENSSL_atomic_add is not declared. The reason is that so far it was
> reserved for occasion like this, i.e. when somebody requests it and puts
> effort into testing it in real-life application:-) Can we do that?
> Original idea behind OPENSSL_atomic_add was even to enable it
> automatically. On related note one can also add pointer manipulation
> subroutines so that one can maintain linked lists in lock-free manner...
>
> I'll have closer look at second patch and comment later on.
>
> __
> OpenSSL Project http://www.openssl.org
> Development Mailing List   openssl-dev@openssl.org
> Automated List Manager   majord...@openssl.org
>
-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Re: Multithreading performance patches

[Andrey Kulikov]

Hi Fedor,

Thanks for valuable contribution!
About your second patch: could you please advice, what the intended way
to use BIO_TYPE_NO_EX_DATA in real-life applications?
For example TSL-server. Do you have any numbers on performance gain when
using this approach?

Re: Multithreading performance patches

[Andy Polyakov]

 Wow, indeed my first patch is useless taking in account presence
 of OPENSSL_atomic_add and locking callback.

Why would you say something like this? It's very valuable input!

 Surely, it would be good if
 openssl will export this symbol, this is very important for tlsnappy
 since it's compiling to shared library and non-exported symbols are not
 visible for it.

You didn't say anything about your platform, but by default it should be
visible on Unix. But one can as well argue that application shouldn't
have to care about it, instead one can set the callback from withing
libcrypto, so that symbol in question doesn't have to be exported. So
assuming that you run on x86[_64] modify OPENSSL_cpuid_setup in
crypto/cryptlib.c, by declaring int OPENSSL_atomic_add(int
*,int,int,void*,int) and adding
CRYPTO_set_add_lock_callback(OPENSSL_atomic_add) at the end of
OPENSSL_cpuid_setup. Questions are if your application operates
correctly, and if it delivers expected performance (keep the other patch
applied).
__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org

Re: Multithreading performance patches

[Andy Polyakov]

 I'm working on fast multithreaded TLS server for node.js, and I've
 encountered few problems with openssl threading support which I would
 like to address with attached patches.
 
 Please ask me if you have any questions or comments.

As for 1st patch. Note that the section in question is guarded by 'if
(add_lock_callback)'. add_lock_callback can be set by application and
perform operation to its liking. In addition, on platforms with
OPENSSL_CPUID_OBJ defined there is OPENSSL_atomic_add that can be used
as callback, so that all you need to do then is
CRYPTO_set_add_lock_callback(OPENSSL_atomic_add) in application code.
Well, you'll surely get into trouble compiling this line, because
OPENSSL_atomic_add is not declared. The reason is that so far it was
reserved for occasion like this, i.e. when somebody requests it and puts
effort into testing it in real-life application:-) Can we do that?
Original idea behind OPENSSL_atomic_add was even to enable it
automatically. On related note one can also add pointer manipulation
subroutines so that one can maintain linked lists in lock-free manner...

I'll have closer look at second patch and comment later on.

__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org

Re: Multithreading performance patches

[Fedor Indutny]

Wow, indeed my first patch is useless taking in account presence of
OPENSSL_atomic_add
and locking callback. Surely, it would be good if openssl will export this
symbol, this is very important for tlsnappy since it's compiling to shared
library and non-exported symbols are not visible for it.




On Fri, Oct 12, 2012 at 10:19 PM, Andy Polyakov ap...@openssl.org wrote:

 OPENSSL_atomic_add

Multithreading performance patches

[Fedor Indutny]

Hey people!

I'm working on fast multithreaded TLS server for node.js, and I've
encountered few problems with openssl threading support which I would like
to address with attached patches.

Please ask me if you have any questions or comments.

Cheers,
Fedor.


1.patch
Description: Binary data


2.patch
Description: Binary data