[DTrace-devel] [PATCH v3 09/11] htab reduction: providers
Nick Alcock
nick.alcock at oracle.com
Mon Nov 1 14:29:45 UTC 2021
Providers were stored in fixed-size hash tables and companion
lists. The fixed-size tables generally were grossly oversized;
with USDT or pid, they were terribly undersized.
Eliminate the lists and replace the hash tables with dt_htab.
We can move to using destructors (allowing deletion of htab elements
without needing a specialized _destroy function, and straightforward
freeing of the dt_provs htab on dtrace_close), but this is a little
troublesome because the dt_provlist is separate from dt_provs, and since
dt_provs element destructors don't have access to a dtp, the destructors
also can't get at the dt_provlist to unchain dt_provs elements that are
being deleted.
We can fix all this by dropping the dt_provlist and replacing it with
dt_htab_next-based iteration over dt_provs. The iteration order is
different, but that's all that changes.
(This is... debatable, and I can see why people might prefer to keep the
old design and just keep a dtp backpointer in every provider element.
I'm happy with that approach too.)
Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
---
libdtrace/dt_dof.c | 5 ++-
libdtrace/dt_impl.h | 5 +--
libdtrace/dt_open.c | 14 ++----
libdtrace/dt_pcb.c | 8 ++--
libdtrace/dt_probe.c | 4 +-
libdtrace/dt_program.c | 8 ++--
libdtrace/dt_provider.c | 97 +++++++++++++++++++++++------------------
libdtrace/dt_provider.h | 3 +-
8 files changed, 75 insertions(+), 69 deletions(-)
diff --git a/libdtrace/dt_dof.c b/libdtrace/dt_dof.c
index 06317d462d13..947d47e58897 100644
--- a/libdtrace/dt_dof.c
+++ b/libdtrace/dt_dof.c
@@ -746,8 +746,9 @@ dtrace_dof_create(dtrace_hdl_t *dtp, dtrace_prog_t *pgp, uint_t flags)
* to the providers and the probes and arguments that they define.
*/
if (flags & DTRACE_D_PROBES) {
- for (pvp = dt_list_next(&dtp->dt_provlist);
- pvp != NULL; pvp = dt_list_next(pvp))
+ dt_htab_next_t *it = NULL;
+
+ while ((pvp = dt_htab_next(dtp->dt_provs, &it)) != NULL)
dof_add_provider(ddo, pvp);
}
diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
index e4611908d12c..615cf0964834 100644
--- a/libdtrace/dt_impl.h
+++ b/libdtrace/dt_impl.h
@@ -324,10 +324,7 @@ struct dtrace_hdl {
struct dt_probe *dt_error; /* ERROR probe */
- dt_list_t dt_provlist; /* linked list of dt_provider_t's */
- struct dt_provider **dt_provs; /* hash table of dt_provider_t's */
- uint_t dt_provbuckets; /* number of provider hash buckets */
- uint_t dt_nprovs; /* number of providers in hash and list */
+ dt_htab_t *dt_provs; /* hash table of dt_provider_t's */
const struct dt_provider *dt_prov_pid; /* PID provider */
dt_proc_hash_t *dt_procs; /* hash table of grabbed process handles */
dt_intdesc_t dt_ints[6]; /* cached integer type descriptions */
diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
index 1681940f1f74..c145c1041454 100644
--- a/libdtrace/dt_open.c
+++ b/libdtrace/dt_open.c
@@ -736,8 +736,6 @@ dt_vopen(int version, int flags, int *errp,
dtp->dt_poll_fd = -1;
dtp->dt_modbuckets = _dtrace_strbuckets;
dtp->dt_mods = calloc(dtp->dt_modbuckets, sizeof(dt_module_t *));
- dtp->dt_provbuckets = _dtrace_strbuckets;
- dtp->dt_provs = calloc(dtp->dt_provbuckets, sizeof(dt_provider_t *));
dt_proc_hash_create(dtp);
dtp->dt_proc_fd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);
dtp->dt_nextepid = 1;
@@ -770,10 +768,9 @@ dt_vopen(int version, int flags, int *errp,
if (dt_str2kver(dtp->dt_uts.release, &dtp->dt_kernver) < 0)
return set_open_errno(dtp, errp, EDT_VERSINVAL);
- if (dtp->dt_mods == NULL || dtp->dt_provs == NULL ||
- dtp->dt_procs == NULL || dtp->dt_ld_path == NULL ||
- dtp->dt_cpp_path == NULL || dtp->dt_cpp_argv == NULL ||
- dtp->dt_sysslice == NULL)
+ if (dtp->dt_mods == NULL || dtp->dt_procs == NULL ||
+ dtp->dt_ld_path == NULL || dtp->dt_cpp_path == NULL ||
+ dtp->dt_cpp_argv == NULL || dtp->dt_sysslice == NULL)
return set_open_errno(dtp, errp, EDT_NOMEM);
for (i = 0; i < DTRACEOPT_MAX; i++)
@@ -1183,7 +1180,6 @@ dtrace_close(dtrace_hdl_t *dtp)
{
dt_ident_t *idp, *ndp;
dt_module_t *dmp;
- dt_provider_t *pvp;
dtrace_prog_t *pgp;
dt_xlator_t *dxp;
dt_dirpath_t *dirp;
@@ -1259,8 +1255,7 @@ dtrace_close(dtrace_hdl_t *dtp)
dt_dof_fini(dtp);
dt_probe_fini(dtp);
- while ((pvp = dt_list_next(&dtp->dt_provlist)) != NULL)
- dt_provider_destroy(dtp, pvp);
+ dt_htab_destroy(dtp, dtp->dt_provs);
for (i = 1; i < dtp->dt_cpp_argc; i++)
free(dtp->dt_cpp_argv[i]);
@@ -1282,7 +1277,6 @@ dtrace_close(dtrace_hdl_t *dtp)
free(dtp->dt_mods);
free(dtp->dt_module_path);
- free(dtp->dt_provs);
free(dtp->dt_strtab);
free(dtp);
diff --git a/libdtrace/dt_pcb.c b/libdtrace/dt_pcb.c
index 620e0d0cf80f..9b9ff7e6dde9 100644
--- a/libdtrace/dt_pcb.c
+++ b/libdtrace/dt_pcb.c
@@ -112,7 +112,8 @@ dt_pcb_pop(dtrace_hdl_t *dtp, int err)
if (err != 0) {
dt_xlator_t *dxp, *nxp;
- dt_provider_t *pvp, *nvp;
+ dt_provider_t *pvp;
+ dt_htab_next_t *it = NULL;
if (pcb->pcb_prog != NULL)
dt_program_destroy(dtp, pcb->pcb_prog);
@@ -127,8 +128,7 @@ dt_pcb_pop(dtrace_hdl_t *dtp, int err)
dt_xlator_destroy(dtp, dxp);
}
- for (pvp = dt_list_next(&dtp->dt_provlist); pvp; pvp = nvp) {
- nvp = dt_list_next(pvp);
+ while ((pvp = dt_htab_next(dtp->dt_provs, &it)) != NULL) {
if (pvp->pv_gen == dtp->dt_gen) {
dtrace_probedesc_t pdp;
@@ -141,7 +141,7 @@ dt_pcb_pop(dtrace_hdl_t *dtp, int err)
dt_probe_iter(dtp, &pdp, dt_pcb_destroy_probe,
NULL, NULL);
- dt_provider_destroy(dtp, pvp);
+ dt_htab_delete(dtp->dt_provs, pvp);
}
}
diff --git a/libdtrace/dt_probe.c b/libdtrace/dt_probe.c
index d41253748f03..3594097e2116 100644
--- a/libdtrace/dt_probe.c
+++ b/libdtrace/dt_probe.c
@@ -1061,6 +1061,7 @@ dt_probe_iter(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp,
dt_probe_t tmpl;
dt_probe_t *prp;
dt_provider_t *pvp;
+ dt_htab_next_t *it = NULL;
int i;
int p_is_glob, m_is_glob, f_is_glob, n_is_glob;
int rv = 0;
@@ -1123,8 +1124,7 @@ dt_probe_iter(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp,
/*
* Loop over providers, allowing them to provide these probes.
*/
- for (pvp = dt_list_next(&dtp->dt_provlist); pvp != NULL;
- pvp = dt_list_next(pvp)) {
+ while ((pvp = dt_htab_next(dtp->dt_provs, &it)) != NULL) {
if (pvp->impl->provide == NULL ||
!dt_gmatch(pvp->desc.dtvd_name, pdp->prv))
continue;
diff --git a/libdtrace/dt_program.c b/libdtrace/dt_program.c
index c114fda72081..a95d0eefdb1a 100644
--- a/libdtrace/dt_program.c
+++ b/libdtrace/dt_program.c
@@ -515,6 +515,7 @@ int
dtrace_program_header(dtrace_hdl_t *dtp, FILE *out, const char *fname)
{
dt_provider_t *pvp;
+ dt_htab_next_t *it = NULL;
char *mfname = NULL, *p;
if (fname != NULL) {
@@ -535,10 +536,11 @@ dtrace_program_header(dtrace_hdl_t *dtp, FILE *out, const char *fname)
if (fprintf(out, "#ifdef\t__cplusplus\nextern \"C\" {\n#endif\n\n") < 0)
return -1;
- for (pvp = dt_list_next(&dtp->dt_provlist);
- pvp != NULL; pvp = dt_list_next(pvp)) {
- if (dt_header_provider(dtp, pvp, out) != 0)
+ while ((pvp = dt_htab_next(dtp->dt_provs, &it)) != NULL) {
+ if (dt_header_provider(dtp, pvp, out) != 0) {
+ dt_htab_next_destroy(it);
return -1; /* dt_errno is set for us */
+ }
}
if (fprintf(out, "\n#ifdef\t__cplusplus\n}\n#endif\n") < 0)
diff --git a/libdtrace/dt_provider.c b/libdtrace/dt_provider.c
index 3d0836710503..6fdf7a08f60f 100644
--- a/libdtrace/dt_provider.c
+++ b/libdtrace/dt_provider.c
@@ -22,14 +22,57 @@
#include <dt_string.h>
#include <dt_list.h>
+static uint32_t
+dt_provider_hval(const dt_provider_t *pvp)
+{
+ return str2hval(pvp->desc.dtvd_name, 0);
+}
+
+static int
+dt_provider_cmp(const dt_provider_t *p,
+ const dt_provider_t *q)
+{
+ return strcmp(p->desc.dtvd_name, q->desc.dtvd_name);
+}
+
+DEFINE_HE_STD_LINK_FUNCS(dt_provider, dt_provider_t, he)
+
+static void *
+dt_provider_del_prov(dt_provider_t *head, dt_provider_t *pvp)
+{
+ head = dt_provider_del(head, pvp);
+
+ if (pvp->pv_probes != NULL)
+ dt_idhash_destroy(pvp->pv_probes);
+
+ dt_node_link_free(&pvp->pv_nodes);
+ free(pvp->pv_xrefs);
+ free(pvp);
+
+ return head;
+}
+
+static dt_htab_ops_t dt_provider_htab_ops = {
+ .hval = (htab_hval_fn)dt_provider_hval,
+ .cmp = (htab_cmp_fn)dt_provider_cmp,
+ .add = (htab_add_fn)dt_provider_add,
+ .next = (htab_next_fn)dt_provider_next,
+ .del = (htab_del_fn)dt_provider_del_prov
+};
+
static dt_provider_t *
-dt_provider_insert(dtrace_hdl_t *dtp, dt_provider_t *pvp, uint_t h)
+dt_provider_insert(dtrace_hdl_t *dtp, dt_provider_t *pvp)
{
- dt_list_append(&dtp->dt_provlist, pvp);
+ if (!dtp->dt_provs) {
+ dtp->dt_provs = dt_htab_create(dtp, &dt_provider_htab_ops);
+ if (dtp->dt_provs == NULL)
+ return NULL;
+ }
- pvp->pv_next = dtp->dt_provs[h];
- dtp->dt_provs[h] = pvp;
- dtp->dt_nprovs++;
+ if (dt_htab_insert(dtp->dt_provs, pvp) < 0) {
+ free(pvp);
+ return NULL;
+ }
return pvp;
}
@@ -37,15 +80,13 @@ dt_provider_insert(dtrace_hdl_t *dtp, dt_provider_t *pvp, uint_t h)
dt_provider_t *
dt_provider_lookup(dtrace_hdl_t *dtp, const char *name)
{
- uint_t h = str2hval(name, 0) % dtp->dt_provbuckets;
- dt_provider_t *pvp;
+ dt_provider_t tmpl;
- for (pvp = dtp->dt_provs[h]; pvp != NULL; pvp = pvp->pv_next) {
- if (strcmp(pvp->desc.dtvd_name, name) == 0)
- return pvp;
- }
+ if ((strlen(name) + 1) > sizeof(tmpl.desc.dtvd_name))
+ return NULL;
- return NULL;
+ strcpy(tmpl.desc.dtvd_name, name);
+ return dt_htab_lookup(dtp->dt_provs, &tmpl);
}
dt_provider_t *
@@ -62,6 +103,7 @@ dt_provider_create(dtrace_hdl_t *dtp, const char *name,
pvp->pv_probes = dt_idhash_create(pvp->desc.dtvd_name, NULL, 0, 0);
pvp->pv_gen = dtp->dt_gen;
pvp->pv_hdl = dtp;
+ dt_dprintf("creating provider %s\n", name);
if (pvp->pv_probes == NULL) {
dt_free(dtp, pvp);
@@ -71,36 +113,7 @@ dt_provider_create(dtrace_hdl_t *dtp, const char *name,
memcpy(&pvp->desc.dtvd_attr, pattr, sizeof(dtrace_pattr_t));
- return dt_provider_insert(dtp, pvp,
- str2hval(name, 0) % dtp->dt_provbuckets);
-}
-
-void
-dt_provider_destroy(dtrace_hdl_t *dtp, dt_provider_t *pvp)
-{
- dt_provider_t **pp;
- uint_t h;
-
- assert(pvp->pv_hdl == dtp);
-
- h = str2hval(pvp->desc.dtvd_name, 0) % dtp->dt_provbuckets;
- pp = &dtp->dt_provs[h];
-
- while (*pp != NULL && *pp != pvp)
- pp = &(*pp)->pv_next;
-
- assert(*pp != NULL && *pp == pvp);
- *pp = pvp->pv_next;
-
- dt_list_delete(&dtp->dt_provlist, pvp);
- dtp->dt_nprovs--;
-
- if (pvp->pv_probes != NULL)
- dt_idhash_destroy(pvp->pv_probes);
-
- dt_node_link_free(&pvp->pv_nodes);
- dt_free(dtp, pvp->pv_xrefs);
- dt_free(dtp, pvp);
+ return dt_provider_insert(dtp, pvp);
}
int
diff --git a/libdtrace/dt_provider.h b/libdtrace/dt_provider.h
index a24b70f9f9d3..53193e28889b 100644
--- a/libdtrace/dt_provider.h
+++ b/libdtrace/dt_provider.h
@@ -87,7 +87,7 @@ extern dt_provimpl_t dt_syscall;
typedef struct dt_provider {
dt_list_t pv_list; /* list forward/back pointers */
- struct dt_provider *pv_next; /* pointer to next provider in hash */
+ struct dt_hentry he; /* htab links */
dtrace_providerdesc_t desc; /* provider name and attributes */
const dt_provimpl_t *impl; /* provider implementation */
dt_idhash_t *pv_probes; /* probe defs (if user-declared) */
@@ -127,7 +127,6 @@ extern dt_provider_t *dt_provider_lookup(dtrace_hdl_t *, const char *);
extern dt_provider_t *dt_provider_create(dtrace_hdl_t *, const char *,
const dt_provimpl_t *,
const dtrace_pattr_t *);
-extern void dt_provider_destroy(dtrace_hdl_t *, dt_provider_t *);
extern int dt_provider_xref(dtrace_hdl_t *, dt_provider_t *, id_t);
#ifdef __cplusplus
--
2.33.1.257.g9e0974a4e8
More information about the DTrace-devel
mailing list