Skip to content

Commit

Permalink
Decoder resolution performance optimizations
Browse files Browse the repository at this point in the history
This refactors decoder functionality to reduce calls to
OSSL_DECODER_is_a / EVP_KEYMGMT_is_a, which are substantial bottlenecks
in the performance of repeated decode operations (see #15199).

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #17921)

(cherry picked from commit 2475544)
  • Loading branch information
hlandau authored and t8m committed Nov 11, 2022
1 parent ae15484 commit 4a1108e
Show file tree
Hide file tree
Showing 4 changed files with 222 additions and 143 deletions.
20 changes: 14 additions & 6 deletions crypto/encode_decode/decoder_lib.c
Expand Up @@ -18,6 +18,7 @@
#include <openssl/trace.h>
#include "internal/bio.h"
#include "internal/provider.h"
#include "internal/namemap.h"
#include "crypto/decoder.h"
#include "encoder_local.h"
#include "internal/e_os.h"
Expand Down Expand Up @@ -241,6 +242,7 @@ OSSL_DECODER_INSTANCE *ossl_decoder_instance_new(OSSL_DECODER *decoder,
/* The "input" property is mandatory */
prop = ossl_property_find_property(props, libctx, "input");
decoder_inst->input_type = ossl_property_get_string_value(libctx, prop);
decoder_inst->input_type_id = 0;
if (decoder_inst->input_type == NULL) {
ERR_raise_data(ERR_LIB_OSSL_DECODER, ERR_R_INVALID_PROPERTY_DEFINITION,
"the mandatory 'input' property is missing "
Expand Down Expand Up @@ -343,6 +345,8 @@ int OSSL_DECODER_CTX_add_decoder(OSSL_DECODER_CTX *ctx, OSSL_DECODER *decoder)
struct collect_extra_decoder_data_st {
OSSL_DECODER_CTX *ctx;
const char *output_type;
int output_type_id;

/*
* 0 to check that the decoder's input type is the same as the decoder name
* 1 to check that the decoder's input type differs from the decoder name
Expand Down Expand Up @@ -370,7 +374,7 @@ static void collect_extra_decoder(OSSL_DECODER *decoder, void *arg)
const OSSL_PROVIDER *prov = OSSL_DECODER_get0_provider(decoder);
void *provctx = OSSL_PROVIDER_get0_provider_ctx(prov);

if (OSSL_DECODER_is_a(decoder, data->output_type)) {
if (ossl_decoder_fast_is_a(decoder, data->output_type, &data->output_type_id)) {
void *decoderctx = NULL;
OSSL_DECODER_INSTANCE *di = NULL;

Expand Down Expand Up @@ -413,8 +417,9 @@ static void collect_extra_decoder(OSSL_DECODER *decoder, void *arg)
switch (data->type_check) {
case IS_SAME:
/* If it differs, this is not a decoder to add for now. */
if (!OSSL_DECODER_is_a(decoder,
OSSL_DECODER_INSTANCE_get_input_type(di))) {
if (!ossl_decoder_fast_is_a(decoder,
OSSL_DECODER_INSTANCE_get_input_type(di),
&di->input_type_id)) {
ossl_decoder_instance_free(di);
OSSL_TRACE_BEGIN(DECODER) {
BIO_printf(trc_out,
Expand All @@ -425,8 +430,9 @@ static void collect_extra_decoder(OSSL_DECODER *decoder, void *arg)
break;
case IS_DIFFERENT:
/* If it's the same, this is not a decoder to add for now. */
if (OSSL_DECODER_is_a(decoder,
OSSL_DECODER_INSTANCE_get_input_type(di))) {
if (ossl_decoder_fast_is_a(decoder,
OSSL_DECODER_INSTANCE_get_input_type(di),
&di->input_type_id)) {
ossl_decoder_instance_free(di);
OSSL_TRACE_BEGIN(DECODER) {
BIO_printf(trc_out,
Expand Down Expand Up @@ -534,6 +540,7 @@ int OSSL_DECODER_CTX_add_extra(OSSL_DECODER_CTX *ctx,
data.output_type
= OSSL_DECODER_INSTANCE_get_input_type(decoder_inst);

data.output_type_id = 0;

for (j = 0; j < numdecoders; j++)
collect_extra_decoder(sk_OSSL_DECODER_value(skdecoders, j),
Expand Down Expand Up @@ -867,7 +874,8 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
* |new_input_type| holds the value of the "input-type" parameter
* for the decoder we're currently considering.
*/
if (decoder != NULL && !OSSL_DECODER_is_a(decoder, new_input_type)) {
if (decoder != NULL && !ossl_decoder_fast_is_a(decoder, new_input_type,
&new_decoder_inst->input_type_id)) {
OSSL_TRACE_BEGIN(DECODER) {
BIO_printf(trc_out,
"(ctx %p) %s [%u] the input type doesn't match the name of the previous decoder (%p), skipping...\n",
Expand Down
18 changes: 18 additions & 0 deletions crypto/encode_decode/decoder_meth.c
Expand Up @@ -558,6 +558,24 @@ int OSSL_DECODER_is_a(const OSSL_DECODER *decoder, const char *name)
return 0;
}

static int resolve_name(OSSL_DECODER *decoder, const char *name)
{
OSSL_LIB_CTX *libctx = ossl_provider_libctx(decoder->base.prov);
OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx);

return ossl_namemap_name2num(namemap, name);
}

int ossl_decoder_fast_is_a(OSSL_DECODER *decoder, const char *name, int *id_cache)
{
int id = *id_cache;

if (id <= 0)
*id_cache = id = resolve_name(decoder, name);

return id > 0 && ossl_decoder_get_number(decoder) == id;
}

struct do_one_data_st {
void (*user_fn)(OSSL_DECODER *decoder, void *arg);
void *user_arg;
Expand Down

0 comments on commit 4a1108e

Please sign in to comment.