[DTrace-devel] [PATCH v2 09/11] htab reduction: providers
Nick Alcock
nick.alcock at oracle.com
Wed Oct 20 11:54:02 PDT 2021
The list of providers is a hand-rolled fixed-size htab which is grossly
oversized except when USDT or the pid provider is in use, when it can
suddenly become terribly undersized: replace it with a 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,
inaccessible from dt_provs element destructors (which do not have access
to a dtp), yet its members are added/removed from the dtp as we go.
We can fix 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 | 4 +-
libdtrace/dt_impl.h | 4 +-
libdtrace/dt_open.c | 14 ++----
libdtrace/dt_pcb.c | 8 ++--
libdtrace/dt_probe.c | 4 +-
libdtrace/dt_program.c | 8 ++--
libdtrace/dt_provider.c | 96 +++++++++++++++++++++++------------------
libdtrace/dt_provider.h | 3 +-
8 files changed, 74 insertions(+), 67 deletions(-)
diff --git a/libdtrace/dt_dof.c b/libdtrace/dt_dof.c
index 06317d462d13..4b5ef877200d 100644
--- a/libdtrace/dt_dof.c
+++ b/libdtrace/dt_dof.c
@@ -560,6 +560,7 @@ dtrace_dof_create(dtrace_hdl_t *dtp, dtrace_prog_t *pgp, uint_t flags)
uint_t maxfmt = 0;
#endif
+ dt_htab_next_t *it = NULL;
dt_provider_t *pvp;
dt_xlator_t *dxp;
dof_actdesc_t *dofa;
@@ -746,8 +747,7 @@ 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))
+ 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 36a445270b11..e8d1fcbfa279 100644
--- a/libdtrace/dt_impl.h
+++ b/libdtrace/dt_impl.h
@@ -324,9 +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 */
+ dt_htab_t *dt_provs; /* hash table of dt_provider_t's */
uint_t dt_nprovs; /* number of providers in hash and list */
const struct dt_provider *dt_prov_pid; /* PID provider */
dt_proc_hash_t *dt_procs; /* hash table of grabbed process handles */
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..6b4a5cb8ad60 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_next_delete(it);
}
}
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..a16104038d25 100644
--- a/libdtrace/dt_provider.c
+++ b/libdtrace/dt_provider.c
@@ -22,13 +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;
+ if (dt_htab_insert(dtp->dt_provs, pvp) < 0) {
+ free(pvp);
+ return NULL;
+ }
dtp->dt_nprovs++;
return pvp;
@@ -37,15 +81,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 +104,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 +114,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