htab reduction: providers

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.

Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
Reviewed-by: Eugene Loh <eugene.loh@oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

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);
 	}
 

libdtrace/dt_impl.h

@@ -323,10 +323,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 */

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++)
@@ -1182,7 +1179,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;
@@ -1258,8 +1254,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]);
@@ -1281,7 +1276,6 @@ dtrace_close(dtrace_hdl_t *dtp)
 
 	free(dtp->dt_mods);
 	free(dtp->dt_module_path);
-	free(dtp->dt_provs);
 	free(dtp);
 
 	dt_debug_dump(0);

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);
 			}
 		}
 

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;

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)

libdtrace/dt_provider.c

@@ -22,30 +22,71 @@
 #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,
+	.del = (htab_del_fn)dt_provider_del_prov,
+	.next = (htab_next_fn)dt_provider_next
+};
+
 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;
 }
 
 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

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