[DTrace-devel] [PATCH 1/7] htab reduction: kernpath
Nick Alcock
nick.alcock at oracle.com
Tue Oct 5 06:38:45 PDT 2021
We start out reducing hand-rolled htabs by dropping a simple one,
the kernpath hash.
This has very little effect on performance, because even though the htab
is terribly poorly sized, it is sized in the wrong direction: the
kernpath hash only contains out-of-tree modules, and there won't be
hundreds of those. The main improvement here is in clarity.
---
cmd/ctf_module_dump.c | 6 +--
libdtrace/dt_impl.h | 5 +--
libdtrace/dt_kernel_module.c | 80 ++++++++++++++++++++++--------------
libdtrace/dt_open.c | 11 ++---
4 files changed, 56 insertions(+), 46 deletions(-)
diff --git a/cmd/ctf_module_dump.c b/cmd/ctf_module_dump.c
index b31110432e05..c927afc171bc 100644
--- a/cmd/ctf_module_dump.c
+++ b/cmd/ctf_module_dump.c
@@ -290,12 +290,9 @@ main(int argc, char *argv[])
* Construct a skeletal dtrace_hdl, enough to make the module path
* searching code happy.
*/
- dtp->dt_kernpathbuckets = BUCKETS;
- dtp->dt_kernpaths = calloc(dtp->dt_kernpathbuckets, sizeof(dt_kern_path_t *));
dtp->dt_module_path = strdup(modpath);
- if (dtp->dt_kernpaths == NULL ||
- dtp->dt_module_path == NULL) {
+ if (dtp->dt_module_path == NULL) {
fprintf(stderr, "Out of memory\n");
exit(1);
}
@@ -306,7 +303,6 @@ main(int argc, char *argv[])
while ((dkpp = dt_list_next(&dtp->dt_kernpathlist)) != NULL)
dtrace__internal_kern_path_destroy(dtp, dkpp);
- free(dtp->dt_kernpaths);
free(revno);
free(module_root);
diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
index 812d5d745f9b..120f437719de 100644
--- a/libdtrace/dt_impl.h
+++ b/libdtrace/dt_impl.h
@@ -160,7 +160,7 @@ typedef struct dt_module {
*/
typedef struct dt_kern_path {
dt_list_t dkp_list; /* list forward/back pointers */
- struct dt_kern_path *dkp_next; /* hash chain next pointer */
+ struct dt_hentry dkp_he; /* htab links */
char *dkp_name; /* name of this kernel module */
char *dkp_path; /* full name including path */
} dt_kern_path_t;
@@ -289,8 +289,7 @@ struct dtrace_hdl {
char *dt_ctfa_path; /* path to vmlinux.ctfa */
const dt_modops_t *dt_ctf_ops; /* data model's ops vector for CTF module */
dt_list_t dt_kernpathlist; /* linked list of dt_kern_path_t's */
- dt_kern_path_t **dt_kernpaths; /* hash table of dt_kern_path_t's */
- uint_t dt_kernpathbuckets; /* number of kernel module path hash buckets */
+ dt_htab_t *dt_kernpaths; /* hash table of dt_kern_path_t's */
uint_t dt_nkernpaths; /* number of kernel module paths in hash and list */
dt_module_t *dt_exec; /* pointer to executable module */
dt_module_t *dt_cdefs; /* pointer to C dynamic type module */
diff --git a/libdtrace/dt_kernel_module.c b/libdtrace/dt_kernel_module.c
index bdd159869bad..0121cfb1c0a0 100644
--- a/libdtrace/dt_kernel_module.c
+++ b/libdtrace/dt_kernel_module.c
@@ -29,6 +29,34 @@
static pthread_mutex_t kern_path_update_lock = PTHREAD_MUTEX_INITIALIZER;
+static uint32_t
+kernpath_hval(const dt_kern_path_t *dkp)
+{
+ return str2hval(dkp->dkp_name, 0);
+}
+
+static int
+kernpath_cmp(const dt_kern_path_t *p,
+ const dt_kern_path_t *q)
+{
+ return strcmp(p->dkp_name, q->dkp_name);
+}
+
+DEFINE_HE_STD_LINK_FUNCS(kernpath, dt_kern_path_t, dkp_he)
+DEFINE_HTAB_STD_OPS(kernpath)
+
+static dt_kern_path_t *
+dt_kern_path_lookup(dtrace_hdl_t *dtp, const char *name)
+{
+ dt_kern_path_t tmpl;
+
+ if (_dt_unlikely_(!dtp->dt_kernpaths))
+ return NULL;
+
+ tmpl.dkp_name = (char *) name;
+ return dt_htab_lookup(dtp->dt_kernpaths, &tmpl);
+}
+
/*
* On successful return, name and path become owned by dt_kern_path_create()
* (and may be freed immediately).
@@ -39,28 +67,34 @@ static pthread_mutex_t kern_path_update_lock = PTHREAD_MUTEX_INITIALIZER;
dt_kern_path_t *
dt_kern_path_create(dtrace_hdl_t *dtp, char *name, char *path)
{
- uint_t h = str2hval(name, 0) % dtp->dt_kernpathbuckets;
dt_kern_path_t *dkpp;
- for (dkpp = dtp->dt_kernpaths[h]; dkpp != NULL; dkpp = dkpp->dkp_next) {
- if (strcmp(dkpp->dkp_name, name) == 0) {
- free(name);
- free(path);
- return dkpp;
- }
+ if (!dtp->dt_kernpaths) {
+ dtp->dt_kernpaths = dt_htab_create(dtp, &kernpath_htab_ops);
+
+ if (!dtp->dt_kernpaths)
+ return NULL; /* caller must handle allocation failure */
+ }
+
+ if ((dkpp = dt_kern_path_lookup(dtp, name)) != NULL) {
+ free(name);
+ free(path);
+ return dkpp;
}
if ((dkpp = malloc(sizeof(dt_kern_path_t))) == NULL)
- return NULL; /* caller must handle allocation failure */
+ return NULL;
dt_dprintf("Adding %s -> %s\n", name, path);
memset(dkpp, 0, sizeof(dt_kern_path_t));
dkpp->dkp_name = name;
dkpp->dkp_path = path; /* strdup()ped by our caller */
+ if (dt_htab_insert(dtp->dt_kernpaths, dkpp) < 0) {
+ free(dkpp);
+ return NULL;
+ }
dt_list_append(&dtp->dt_kernpathlist, dkpp);
- dkpp->dkp_next = dtp->dt_kernpaths[h];
- dtp->dt_kernpaths[h] = dkpp;
dtp->dt_nkernpaths++;
return dkpp;
@@ -69,26 +103,18 @@ dt_kern_path_create(dtrace_hdl_t *dtp, char *name, char *path)
void
dt_kern_path_destroy(dtrace_hdl_t *dtp, dt_kern_path_t *dkpp)
{
- uint_t h = str2hval(dkpp->dkp_name, 0) % dtp->dt_kernpathbuckets;
- dt_kern_path_t *scan_dkpp;
- dt_kern_path_t *prev_dkpp = NULL;
-
dt_list_delete(&dtp->dt_kernpathlist, dkpp);
assert(dtp->dt_nkernpaths != 0);
dtp->dt_nkernpaths--;
- for (scan_dkpp = dtp->dt_kernpaths[h]; (scan_dkpp != NULL) &&
- (scan_dkpp != dkpp); scan_dkpp = scan_dkpp->dkp_next) {
- prev_dkpp = scan_dkpp;
- }
- if (prev_dkpp == NULL)
- dtp->dt_kernpaths[h] = dkpp->dkp_next;
- else
- prev_dkpp->dkp_next = dkpp->dkp_next;
+ dt_htab_delete(dtp->dt_kernpaths, dkpp);
free(dkpp->dkp_name);
free(dkpp->dkp_path);
free(dkpp);
+
+ if (dtp->dt_nkernpaths == 0)
+ dt_htab_destroy(dtp, dtp->dt_kernpaths);
}
/*
@@ -178,9 +204,6 @@ dt_kern_path_update(dtrace_hdl_t *dtp)
dt_kern_path_t *
dt_kern_path_lookup_by_name(dtrace_hdl_t *dtp, const char *name)
{
- uint_t h = str2hval(name, 0) % dtp->dt_kernpathbuckets;
- dt_kern_path_t *dkpp;
-
if (dtp->dt_nkernpaths == 0) {
int dterrno;
@@ -198,12 +221,7 @@ dt_kern_path_lookup_by_name(dtrace_hdl_t *dtp, const char *name)
dtp->dt_nkernpaths);
}
- for (dkpp = dtp->dt_kernpaths[h]; dkpp != NULL; dkpp = dkpp->dkp_next) {
- if (strcmp(dkpp->dkp_name, name) == 0)
- return dkpp;
- }
-
- return NULL;
+ return dt_kern_path_lookup(dtp, name);
}
/*
diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
index d3908c0be05a..1d16c55f650b 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_kernpathbuckets = _dtrace_strbuckets;
- dtp->dt_kernpaths = calloc(dtp->dt_kernpathbuckets, sizeof(dt_kern_path_t *));
dtp->dt_provbuckets = _dtrace_strbuckets;
dtp->dt_provs = calloc(dtp->dt_provbuckets, sizeof(dt_provider_t *));
dt_proc_hash_create(dtp);
@@ -772,10 +770,10 @@ 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_kernpaths == 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_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)
return set_open_errno(dtp, errp, EDT_NOMEM);
for (i = 0; i < DTRACEOPT_MAX; i++)
@@ -1288,7 +1286,6 @@ dtrace_close(dtrace_hdl_t *dtp)
elf_end(dtp->dt_ctf_elf);
free(dtp->dt_mods);
free(dtp->dt_module_path);
- free(dtp->dt_kernpaths);
free(dtp->dt_provs);
free(dtp->dt_strtab);
free(dtp);
--
2.33.0.256.gb827f06fa9
More information about the DTrace-devel
mailing list