Project

General

Profile

Actions

Bug #6900

open

libdtrace could use calloc instead of malloc/bzero

Added by Garrett D'Amore over 6 years ago. Updated over 6 years ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
DTrace
Start date:
2016-04-11
Due date:
% Done:

0%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:

Description

The following patch was sent to me courtesy of Pedro Giffuni at FreeBSD. The claim is that calloc is faster than malloc + bzero. (I have my doubts about this, because in illumos libc, calloc() is just malloc followed by memset. But the FreeBSD implementation may be faster by being able to utilize pre-zerod pages, or even deferring the memory allocation of calloc() by leaving mmap() /dev/zero and waiting for the pages to fault in.

But if the calloc() test is indeed faster on other systems, it may be to their help if libdtrace were to use calloc. (And maybe we should too!) What I don't know is whether the deferred page allocation from such is both worthwhile (if we're going to immediately touch the pages) and doesn't cause other problems with predictability in illumos.

It would be nice for someone from the dtrace team to take a look a this.

Index: lib/libdtrace/common/dt_module.c
===================================================================
--- lib/libdtrace/common/dt_module.c    (revision 296724)
+++ lib/libdtrace/common/dt_module.c    (working copy)
@@ -24,6 +24,7 @@
  */
 /*
  * Copyright (c) 2013, Joyent, Inc.  All rights reserved.
+ * Copyright (c) 2016, Pedro Giffuni.  All rights reserved.
  */

 #include <sys/types.h>
@@ -721,15 +722,14 @@
         return (dt_set_errno(dtp, EDT_CANTLOAD));
     }

-    dmp->dm_libctfp = malloc(sizeof (ctf_file_t *) * arg.dpa_count);
+    dmp->dm_libctfp = calloc(arg.dpa_count, sizeof (ctf_file_t *));
     if (dmp->dm_libctfp == NULL) {
         dt_proc_unlock(dtp, p);
         dt_proc_release(dtp, p);
         return (dt_set_errno(dtp, EDT_NOMEM));
     }
-    bzero(dmp->dm_libctfp, sizeof (ctf_file_t *) * arg.dpa_count);

-    dmp->dm_libctfn = malloc(sizeof (char *) * arg.dpa_count);
+    dmp->dm_libctfn = calloc(arg.dpa_count, sizeof (char *));
     if (dmp->dm_libctfn == NULL) {
         free(dmp->dm_libctfp);
         dt_proc_unlock(dtp, p);
@@ -736,7 +736,6 @@
         dt_proc_release(dtp, p);
         return (dt_set_errno(dtp, EDT_NOMEM));
     }
-    bzero(dmp->dm_libctfn, sizeof (char *) * arg.dpa_count);

     dmp->dm_nctflibs = arg.dpa_count;

@@ -817,7 +816,7 @@
     dmp->dm_nsymbuckets = _dtrace_strbuckets;
     dmp->dm_symfree = 1;        /* first free element is index 1 */

-    dmp->dm_symbuckets = malloc(sizeof (uint_t) * dmp->dm_nsymbuckets);
+    dmp->dm_symbuckets = calloc(dmp->dm_nsymbuckets, sizeof (uint_t));
     dmp->dm_symchains = malloc(sizeof (dt_sym_t) * dmp->dm_nsymelems + 1);

     if (dmp->dm_symbuckets == NULL || dmp->dm_symchains == NULL) {
@@ -825,7 +824,6 @@
         return (dt_set_errno(dtp, EDT_NOMEM));
     }

-    bzero(dmp->dm_symbuckets, sizeof (uint_t) * dmp->dm_nsymbuckets);
     bzero(dmp->dm_symchains, sizeof (dt_sym_t) * dmp->dm_nsymelems + 1);

     /*
Index: lib/libdtrace/common/dt_regset.c
===================================================================
--- lib/libdtrace/common/dt_regset.c    (revision 296724)
+++ lib/libdtrace/common/dt_regset.c    (working copy)
@@ -23,6 +23,8 @@
 /*
  * Copyright 2003 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
+ *
+ * Portions Copyright 2016 Pedro Giffuni.  All rights reserved.
  */

 /*
@@ -47,8 +49,7 @@
     if (drp == NULL)
         return (NULL);

-    drp->dr_bitmap = malloc(sizeof (ulong_t) * n);
-    drp->dr_size = nregs;
+    drp->dr_bitmap = calloc(n, sizeof (ulong_t));

     if (drp->dr_bitmap == NULL) {
         dt_regset_destroy(drp);
@@ -55,7 +56,8 @@
         return (NULL);
     }

-    bzero(drp->dr_bitmap, sizeof (ulong_t) * n);
+    drp->dr_size = nregs;
+
     return (drp);
 }

Index: lib/libdtrace/common/dt_strtab.c
===================================================================
--- lib/libdtrace/common/dt_strtab.c    (revision 296724)
+++ lib/libdtrace/common/dt_strtab.c    (working copy)
@@ -22,6 +22,8 @@
 /*
  * Copyright 2006 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
+ *
+ * Portions Copyright 2016 Pedro Giffuni.  All rights reserved.
  */

 #pragma ident    "%Z%%M%    %I%    %E% SMI" 
@@ -70,12 +72,11 @@
         return (NULL);

     bzero(sp, sizeof (dt_strtab_t));
-    sp->str_hash = malloc(nbuckets * sizeof (dt_strhash_t *));
+    sp->str_hash = calloc(nbuckets, sizeof (dt_strhash_t *));

     if (sp->str_hash == NULL)
         goto err;

-    bzero(sp->str_hash, nbuckets * sizeof (dt_strhash_t *));
     sp->str_hashsz = nbuckets;
     sp->str_bufs = NULL;
     sp->str_ptr = NULL;
Actions #1

Updated by Josef Sipek over 6 years ago

  • Description updated (diff)
Actions #2

Updated by Josef Sipek over 6 years ago

Personally, I think using calloc is better simply because it's more expressive - seeing calloc tells me the intent faster than seeing malloc and having to look for a possible bzero (or memset) later on.

The diff looks fine. (I'm not sure why the dr_size assignment in dt_regset.c moved. Of course this needs a run through git-pbchk.)

Actions

Also available in: Atom PDF