Remove statistics tracking from LHASH

Fixes #17928. Supercedes #17931.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/17935)
This commit is contained in:
Hugo Landau 2022-03-22 10:59:36 +00:00 committed by Tomas Mraz
parent 2aa645bca4
commit 77d7b6eebb
5 changed files with 28 additions and 99 deletions

View File

@ -114,6 +114,15 @@ breaking changes, and mappings for the large list of deprecated functions.
[Migration guide]: https://github.com/openssl/openssl/tree/master/doc/man7/migration_guide.pod
### Changes between 3.0.2 and 3.0.3
* The functions `OPENSSL_LH_stats` and `OPENSSL_LH_stats_bio` now only report
the `num_items`, `num_nodes` and `num_alloc_nodes` statistics. All other
statistics are no longer supported. For compatibility, these statistics are
still listed in the output but are now always reported as zero.
*Hugo Landau*
### Changes between 3.0.1 and 3.0.2 [15 mar 2022]
* Fixed a bug in the BN_mod_sqrt() function that can cause it to loop forever

View File

@ -61,37 +61,22 @@ void OPENSSL_LH_node_usage_stats(const OPENSSL_LHASH *lh, FILE *fp)
void OPENSSL_LH_stats_bio(const OPENSSL_LHASH *lh, BIO *out)
{
int omit_tsan = 0;
#ifdef TSAN_REQUIRES_LOCKING
if (!CRYPTO_THREAD_read_lock(lh->tsan_lock)) {
BIO_printf(out, "unable to lock table, omitting TSAN counters\n");
omit_tsan = 1;
}
#endif
BIO_printf(out, "num_items = %lu\n", lh->num_items);
BIO_printf(out, "num_nodes = %u\n", lh->num_nodes);
BIO_printf(out, "num_alloc_nodes = %u\n", lh->num_alloc_nodes);
BIO_printf(out, "num_expands = %lu\n", lh->num_expands);
BIO_printf(out, "num_expand_reallocs = %lu\n", lh->num_expand_reallocs);
BIO_printf(out, "num_contracts = %lu\n", lh->num_contracts);
BIO_printf(out, "num_contract_reallocs = %lu\n", lh->num_contract_reallocs);
if (!omit_tsan) {
BIO_printf(out, "num_hash_calls = %lu\n", lh->num_hash_calls);
BIO_printf(out, "num_comp_calls = %lu\n", lh->num_comp_calls);
}
BIO_printf(out, "num_insert = %lu\n", lh->num_insert);
BIO_printf(out, "num_replace = %lu\n", lh->num_replace);
BIO_printf(out, "num_delete = %lu\n", lh->num_delete);
BIO_printf(out, "num_no_delete = %lu\n", lh->num_no_delete);
if (!omit_tsan) {
BIO_printf(out, "num_retrieve = %lu\n", lh->num_retrieve);
BIO_printf(out, "num_retrieve_miss = %lu\n", lh->num_retrieve_miss);
BIO_printf(out, "num_hash_comps = %lu\n", lh->num_hash_comps);
#ifdef TSAN_REQUIRES_LOCKING
CRYPTO_THREAD_unlock(lh->tsan_lock);
#endif
}
BIO_printf(out, "num_expands = 0\n");
BIO_printf(out, "num_expand_reallocs = 0\n");
BIO_printf(out, "num_contracts = 0\n");
BIO_printf(out, "num_contract_reallocs = 0\n");
BIO_printf(out, "num_hash_calls = 0\n");
BIO_printf(out, "num_comp_calls = 0\n");
BIO_printf(out, "num_insert = 0\n");
BIO_printf(out, "num_replace = 0\n");
BIO_printf(out, "num_delete = 0\n");
BIO_printf(out, "num_no_delete = 0\n");
BIO_printf(out, "num_retrieve = 0\n");
BIO_printf(out, "num_retrieve_miss = 0\n");
BIO_printf(out, "num_hash_comps = 0\n");
}
void OPENSSL_LH_node_stats_bio(const OPENSSL_LHASH *lh, BIO *out)

View File

@ -44,22 +44,6 @@ static int expand(OPENSSL_LHASH *lh);
static void contract(OPENSSL_LHASH *lh);
static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh, const void *data, unsigned long *rhash);
static ossl_inline int tsan_lock(const OPENSSL_LHASH *lh)
{
#ifdef TSAN_REQUIRES_LOCKING
if (!CRYPTO_THREAD_write_lock(lh->tsan_lock))
return 0;
#endif
return 1;
}
static ossl_inline void tsan_unlock(const OPENSSL_LHASH *lh)
{
#ifdef TSAN_REQUIRES_LOCKING
CRYPTO_THREAD_unlock(lh->tsan_lock);
#endif
}
OPENSSL_LHASH *OPENSSL_LH_new(OPENSSL_LH_HASHFUNC h, OPENSSL_LH_COMPFUNC c)
{
OPENSSL_LHASH *ret;
@ -74,10 +58,6 @@ OPENSSL_LHASH *OPENSSL_LH_new(OPENSSL_LH_HASHFUNC h, OPENSSL_LH_COMPFUNC c)
}
if ((ret->b = OPENSSL_zalloc(sizeof(*ret->b) * MIN_NODES)) == NULL)
goto err;
#ifdef TSAN_REQUIRES_LOCKING
if ((ret->tsan_lock = CRYPTO_THREAD_lock_new()) == NULL)
goto err;
#endif
ret->comp = ((c == NULL) ? (OPENSSL_LH_COMPFUNC)strcmp : c);
ret->hash = ((h == NULL) ? (OPENSSL_LH_HASHFUNC)OPENSSL_LH_strhash : h);
ret->num_nodes = MIN_NODES / 2;
@ -99,9 +79,6 @@ void OPENSSL_LH_free(OPENSSL_LHASH *lh)
return;
OPENSSL_LH_flush(lh);
#ifdef TSAN_REQUIRES_LOCKING
CRYPTO_THREAD_lock_free(lh->tsan_lock);
#endif
OPENSSL_free(lh->b);
OPENSSL_free(lh);
}
@ -147,12 +124,10 @@ void *OPENSSL_LH_insert(OPENSSL_LHASH *lh, void *data)
nn->hash = hash;
*rn = nn;
ret = NULL;
lh->num_insert++;
lh->num_items++;
} else { /* replace same key */
ret = (*rn)->data;
(*rn)->data = data;
lh->num_replace++;
}
return ret;
}
@ -167,14 +142,12 @@ void *OPENSSL_LH_delete(OPENSSL_LHASH *lh, const void *data)
rn = getrn(lh, data, &hash);
if (*rn == NULL) {
lh->num_no_delete++;
return NULL;
} else {
nn = *rn;
*rn = nn->next;
ret = nn->data;
OPENSSL_free(nn);
lh->num_delete++;
}
lh->num_items--;
@ -190,18 +163,11 @@ void *OPENSSL_LH_retrieve(OPENSSL_LHASH *lh, const void *data)
unsigned long hash;
OPENSSL_LH_NODE **rn;
/*-
* This should be atomic without tsan.
* It's not clear why it was done this way and not elsewhere.
*/
tsan_store((TSAN_QUALIFIER int *)&lh->error, 0);
if (lh->error != 0)
lh->error = 0;
rn = getrn(lh, data, &hash);
if (tsan_lock(lh)) {
tsan_counter(*rn == NULL ? &lh->num_retrieve_miss : &lh->num_retrieve);
tsan_unlock(lh);
}
return *rn == NULL ? NULL : (*rn)->data;
}
@ -262,14 +228,12 @@ static int expand(OPENSSL_LHASH *lh)
memset(n + nni, 0, sizeof(*n) * (j - nni));
lh->pmax = nni;
lh->num_alloc_nodes = j;
lh->num_expand_reallocs++;
lh->p = 0;
} else {
lh->p++;
}
lh->num_nodes++;
lh->num_expands++;
n1 = &(lh->b[p]);
n2 = &(lh->b[p + pmax]);
*n2 = NULL;
@ -302,7 +266,6 @@ static void contract(OPENSSL_LHASH *lh)
lh->error++;
return;
}
lh->num_contract_reallocs++;
lh->num_alloc_nodes /= 2;
lh->pmax /= 2;
lh->p = lh->pmax - 1;
@ -311,7 +274,6 @@ static void contract(OPENSSL_LHASH *lh)
lh->p--;
lh->num_nodes--;
lh->num_contracts++;
n1 = lh->b[(int)lh->p];
if (n1 == NULL)
@ -329,14 +291,8 @@ static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh,
OPENSSL_LH_NODE **ret, *n1;
unsigned long hash, nn;
OPENSSL_LH_COMPFUNC cf;
int do_tsan = 1;
#ifdef TSAN_REQUIRES_LOCKING
do_tsan = tsan_lock(lh);
#endif
hash = (*(lh->hash)) (data);
if (do_tsan)
tsan_counter(&lh->num_hash_calls);
*rhash = hash;
nn = hash % lh->pmax;
@ -346,20 +302,14 @@ static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh,
cf = lh->comp;
ret = &(lh->b[(int)nn]);
for (n1 = *ret; n1 != NULL; n1 = n1->next) {
if (do_tsan)
tsan_counter(&lh->num_hash_comps);
if (n1->hash != hash) {
ret = &(n1->next);
continue;
}
if (do_tsan)
tsan_counter(&lh->num_comp_calls);
if (cf(n1->data, data) == 0)
break;
ret = &(n1->next);
}
if (do_tsan)
tsan_unlock(lh);
return ret;
}

View File

@ -27,21 +27,5 @@ struct lhash_st {
unsigned long up_load; /* load times 256 */
unsigned long down_load; /* load times 256 */
unsigned long num_items;
unsigned long num_expands;
unsigned long num_expand_reallocs;
unsigned long num_contracts;
unsigned long num_contract_reallocs;
TSAN_QUALIFIER unsigned long num_hash_calls;
TSAN_QUALIFIER unsigned long num_comp_calls;
unsigned long num_insert;
unsigned long num_replace;
unsigned long num_delete;
unsigned long num_no_delete;
TSAN_QUALIFIER unsigned long num_retrieve;
TSAN_QUALIFIER unsigned long num_retrieve_miss;
TSAN_QUALIFIER unsigned long num_hash_comps;
int error;
#ifdef TSAN_REQUIRES_LOCKING
CRYPTO_RWLOCK *tsan_lock;
#endif
};

View File

@ -23,9 +23,10 @@ OPENSSL_LH_node_stats_bio, OPENSSL_LH_node_usage_stats_bio - LHASH statistics
The B<LHASH> structure records statistics about most aspects of
accessing the hash table.
OPENSSL_LH_stats() prints out statistics on the size of the hash table, how
many entries are in it, and the number and result of calls to the
routines in this library.
OPENSSL_LH_stats() prints out statistics on the size of the hash table and how
many entries are in it. For historical reasons, this function also outputs a
number of additional statistics, but the tracking of these statistics is no
longer supported and these statistics are always reported as zero.
OPENSSL_LH_node_stats() prints the number of entries for each 'bucket' in the
hash table.