Speed up 'zfs list -t snapshot -o name -s name'

FreeBSD #xxx:  Dramatically optimize listing snapshots when user
requests only snapshot names and wants to sort them by name, ie.
when executes:

  # zfs list -t snapshot -o name -s name

Because only name is needed we don't have to read all snapshot
properties.

Below you can find how long does it take to list 34509 snapshots
from a single disk pool before and after this change with cold and
warm cache:

    before:

        # time zfs list -t snapshot -o name -s name > /dev/null
        cold cache: 525s
        warm cache: 218s

    after:

        # time zfs list -t snapshot -o name -s name > /dev/null
        cold cache: 1.7s
        warm cache: 1.1s

NOTE: This patch only appears in FreeBSD.  If/when Illumos picks up
the change we may want to drop this patch and adopt their version.
However, for now this addresses a real issue.

Ported-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue #450

diff --git a/cmd/zfs/zfs_iter.c b/cmd/zfs/zfs_iter.c
index e2ab90eaf14fc5be1234ae068604c0299a543e1d..6239a8f6b93b601d9ccbcce61ad8986144f75b2c 100644 (file)
--- a/cmd/zfs/zfs_iter.c
+++ b/cmd/zfs/zfs_iter.c
@@ -20,6 +20,7 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012 Pawel Jakub Dawidek <pawel@dawidek.net>.
  */
 
 #include <libintl.h>
@@ -129,8 +130,11 @@ zfs_callback(zfs_handle_t *zhp, void *data)
                cb->cb_depth++;
                if (zfs_get_type(zhp) == ZFS_TYPE_FILESYSTEM)
                        (void) zfs_iter_filesystems(zhp, zfs_callback, data);
-               if ((zfs_get_type(zhp) != ZFS_TYPE_SNAPSHOT) && include_snaps)
-                       (void) zfs_iter_snapshots(zhp, zfs_callback, data);
+               if ((zfs_get_type(zhp) != ZFS_TYPE_SNAPSHOT) && include_snaps) {
+                       (void) zfs_iter_snapshots(zhp,
+                           (cb->cb_flags & ZFS_ITER_SIMPLE) != 0, zfs_callback,
+                           data);
+               }
                cb->cb_depth--;
        }
 
@@ -184,6 +188,13 @@ zfs_free_sort_columns(zfs_sort_column_t *sc)
        }
 }
 
+int
+zfs_sort_only_by_name(const zfs_sort_column_t *sc)
+{
+       return (sc != NULL && sc->sc_next == NULL &&
+           sc->sc_prop == ZFS_PROP_NAME);
+}
+
 /* ARGSUSED */
 static int
 zfs_compare(const void *larg, const void *rarg, void *unused)
@@ -224,7 +235,13 @@ zfs_compare(const void *larg, const void *rarg, void *unused)
                        lcreate = zfs_prop_get_int(l, ZFS_PROP_CREATETXG);
                        rcreate = zfs_prop_get_int(r, ZFS_PROP_CREATETXG);
 
-                       if (lcreate < rcreate)
+                       /*
+                        * Both lcreate and rcreate being 0 means we don't have
+                        * properties and we should compare full name.
+                        */
+                       if (lcreate == 0 && rcreate == 0)
+                               ret = strcmp(lat + 1, rat + 1);
+                       else if (lcreate < rcreate)
                                ret = -1;
                        else if (lcreate > rcreate)
                                ret = 1;
@@ -290,7 +307,14 @@ zfs_sort(const void *larg, const void *rarg, void *data)
                        if (rvalid)
                                verify(nvlist_lookup_string(rval,
                                    ZPROP_VALUE, &rstr) == 0);
+               } else if (psc->sc_prop == ZFS_PROP_NAME) {
+                       lvalid = rvalid = B_TRUE;
+
+                       (void) strlcpy(lbuf, zfs_get_name(l), sizeof(lbuf));
+                       (void) strlcpy(rbuf, zfs_get_name(r), sizeof(rbuf));
 
+                       lstr = lbuf;
+                       rstr = rbuf;
                } else if (zfs_prop_is_string(psc->sc_prop)) {
                        lvalid = (zfs_prop_get(l, psc->sc_prop, lbuf,
                            sizeof (lbuf), NULL, NULL, 0, B_TRUE) == 0);

diff --git a/cmd/zfs/zfs_iter.h b/cmd/zfs/zfs_iter.h
index 8c6b9fdef54f0a5e862fc6e7d80f7e323743ac68..7f740e75ef6297f232a84de32193b98a2548dc16 100644 (file)
--- a/cmd/zfs/zfs_iter.h
+++ b/cmd/zfs/zfs_iter.h
@@ -43,11 +43,13 @@ typedef struct zfs_sort_column {
 #define        ZFS_ITER_PROP_LISTSNAPS    (1 << 2)
 #define        ZFS_ITER_DEPTH_LIMIT       (1 << 3)
 #define        ZFS_ITER_RECVD_PROPS       (1 << 4)
+#define        ZFS_ITER_SIMPLE            (1 << 5)
 
 int zfs_for_each(int, char **, int options, zfs_type_t,
     zfs_sort_column_t *, zprop_list_t **, int, zfs_iter_f, void *);
 int zfs_add_sort_column(zfs_sort_column_t **, const char *, boolean_t);
 void zfs_free_sort_columns(zfs_sort_column_t *);
+int zfs_sort_only_by_name(const zfs_sort_column_t *);
 
 #ifdef __cplusplus
 }

diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c
index e20d57000a4433cc62ba84000124c0c2263ff8d4..0f00688b359856d3c0e0d7fbb15460ab859594d7 100644 (file)
--- a/cmd/zfs/zfs_main.c
+++ b/cmd/zfs/zfs_main.c
@@ -2650,7 +2650,12 @@ print_dataset(zfs_handle_t *zhp, zprop_list_t *pl, boolean_t scripted)
                        first = B_FALSE;
                }
 
-               if (pl->pl_prop != ZPROP_INVAL) {
+               if (pl->pl_prop == ZFS_PROP_NAME) {
+                       (void) strlcpy(property, zfs_get_name(zhp),
+                           sizeof(property));
+                       propstr = property;
+                       right_justify = zfs_prop_align_right(pl->pl_prop);
+               } else if (pl->pl_prop != ZPROP_INVAL) {
                        if (zfs_prop_get(zhp, pl->pl_prop, property,
                            sizeof (property), NULL, NULL, 0, B_FALSE) != 0)
                                propstr = "-";
@@ -2810,6 +2815,13 @@ zfs_do_list(int argc, char **argv)
        if (fields == NULL)
                fields = default_fields;
 
+       /*
+        * If we are only going to list snapshot names and sort by name,
+        * then we can use faster version.
+        */
+       if (strcmp(fields, "name") == 0 && zfs_sort_only_by_name(sortcol))
+               flags |= ZFS_ITER_SIMPLE;
+
        /*
         * If "-o space" and no types were specified, don't display snapshots.
         */

diff --git a/include/libzfs.h b/include/libzfs.h
index 26b1ce302b99f1e92f4e2b9f36613c21caad5e19..86e23ac327d11e69d2d393b06d5fcf3e54e9139b 100644 (file)
--- a/include/libzfs.h
+++ b/include/libzfs.h
@@ -516,7 +516,7 @@ extern int zfs_iter_root(libzfs_handle_t *, zfs_iter_f, void *);
 extern int zfs_iter_children(zfs_handle_t *, zfs_iter_f, void *);
 extern int zfs_iter_dependents(zfs_handle_t *, boolean_t, zfs_iter_f, void *);
 extern int zfs_iter_filesystems(zfs_handle_t *, zfs_iter_f, void *);
-extern int zfs_iter_snapshots(zfs_handle_t *, zfs_iter_f, void *);
+extern int zfs_iter_snapshots(zfs_handle_t *, boolean_t, zfs_iter_f, void *);
 extern int zfs_iter_snapshots_sorted(zfs_handle_t *, zfs_iter_f, void *);
 
 typedef struct get_all_cb {

diff --git a/include/sys/zfs_ioctl.h b/include/sys/zfs_ioctl.h
index e3fd2c3fc6eab381b5309c0283c9ad6de022a7a3..4e5c5fba299d162c9a1df3bda1f5c4f26af55e42 100644 (file)
--- a/include/sys/zfs_ioctl.h
+++ b/include/sys/zfs_ioctl.h
@@ -286,7 +286,8 @@ typedef struct zfs_cmd {
        boolean_t       zc_temphold;
        uint64_t        zc_action_handle;
        int             zc_cleanup_fd;
-       uint8_t         zc_pad[4];              /* alignment */
+       uint8_t         zc_simple;
+       uint8_t         zc_pad[3];              /* alignment */
        uint64_t        zc_sendobj;
        uint64_t        zc_fromobj;
        uint64_t        zc_createtxg;

diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c
index 543437492667d5936feb23bbbc285ef5e6b27837..3340c713f8d3bd5f76aae7d58c0e98862778a2c8 100644 (file)
--- a/lib/libzfs/libzfs_dataset.c
+++ b/lib/libzfs/libzfs_dataset.c
@@ -23,6 +23,7 @@
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2010 Nexenta Systems, Inc. All rights reserved.
  * Copyright (c) 2011 by Delphix. All rights reserved.
+ * Copyright (c) 2012 Pawel Jakub Dawidek <pawel@dawidek.net>.
  */
 
 #include <ctype.h>
@@ -480,6 +481,23 @@ make_dataset_handle_zc(libzfs_handle_t *hdl, zfs_cmd_t *zc)
        return (zhp);
 }
 
+static zfs_handle_t *
+make_dataset_simple_handle_zc(zfs_handle_t *pzhp, zfs_cmd_t *zc)
+{
+       zfs_handle_t *zhp = calloc(sizeof (zfs_handle_t), 1);
+
+       if (zhp == NULL)
+               return (NULL);
+
+       zhp->zfs_hdl = pzhp->zfs_hdl;
+       (void) strlcpy(zhp->zfs_name, zc->zc_name, sizeof (zhp->zfs_name));
+       zhp->zfs_head_type = pzhp->zfs_type;
+       zhp->zfs_type = ZFS_TYPE_SNAPSHOT;
+       zhp->zpool_hdl = zpool_handle(zhp);
+
+       return (zhp);
+}
+
 /*
  * Opens the given snapshot, filesystem, or volume.   The 'types'
  * argument is a mask of acceptable types.  The function will print an
@@ -2518,7 +2536,8 @@ zfs_iter_filesystems(zfs_handle_t *zhp, zfs_iter_f func, void *data)
  * Iterate over all snapshots
  */
 int
-zfs_iter_snapshots(zfs_handle_t *zhp, zfs_iter_f func, void *data)
+zfs_iter_snapshots(zfs_handle_t *zhp, boolean_t simple, zfs_iter_f func,
+    void *data)
 {
        zfs_cmd_t zc = { "\0", "\0", "\0", "\0", 0 };
        zfs_handle_t *nzhp;
@@ -2527,15 +2546,19 @@ zfs_iter_snapshots(zfs_handle_t *zhp, zfs_iter_f func, void *data)
        if (zhp->zfs_type == ZFS_TYPE_SNAPSHOT)
                return (0);
 
+       zc.zc_simple = simple;
+
        if (zcmd_alloc_dst_nvlist(zhp->zfs_hdl, &zc, 0) != 0)
                return (-1);
        while ((ret = zfs_do_list_ioctl(zhp, ZFS_IOC_SNAPSHOT_LIST_NEXT,
            &zc)) == 0) {
 
-               if ((nzhp = make_dataset_handle_zc(zhp->zfs_hdl,
-                   &zc)) == NULL) {
+               if (simple)
+                       nzhp = make_dataset_simple_handle_zc(zhp, &zc);
+               else
+                       nzhp = make_dataset_handle_zc(zhp->zfs_hdl, &zc);
+               if (nzhp == NULL)
                        continue;
-               }
 
                if ((ret = func(nzhp, data)) != 0) {
                        zcmd_free_nvlists(&zc);
@@ -2557,7 +2580,7 @@ zfs_iter_children(zfs_handle_t *zhp, zfs_iter_f func, void *data)
        if ((ret = zfs_iter_filesystems(zhp, func, data)) != 0)
                return (ret);
 
-       return (zfs_iter_snapshots(zhp, func, data));
+       return (zfs_iter_snapshots(zhp, B_FALSE, func, data));
 }
 
 /*
@@ -3287,7 +3310,7 @@ zfs_promote(zfs_handle_t *zhp)
                return (-1);
        (void) zfs_prop_get(pzhp, ZFS_PROP_MOUNTPOINT, pd.cb_mountpoint,
            sizeof (pd.cb_mountpoint), NULL, NULL, 0, FALSE);
-       ret = zfs_iter_snapshots(pzhp, promote_snap_cb, &pd);
+       ret = zfs_iter_snapshots(pzhp, B_FALSE, promote_snap_cb, &pd);
        if (ret != 0) {
                zfs_close(pzhp);
                return (-1);
@@ -3302,7 +3325,8 @@ zfs_promote(zfs_handle_t *zhp)
        if (ret != 0) {
                int save_errno = errno;
 
-               (void) zfs_iter_snapshots(pzhp, promote_snap_done_cb, &pd);
+               (void) zfs_iter_snapshots(pzhp, B_FALSE, promote_snap_done_cb,
+                   &pd);
                zfs_close(pzhp);
 
                switch (save_errno) {
@@ -3321,7 +3345,8 @@ zfs_promote(zfs_handle_t *zhp)
                        return (zfs_standard_error(hdl, save_errno, errbuf));
                }
        } else {
-               (void) zfs_iter_snapshots(zhp, promote_snap_done_cb, &pd);
+               (void) zfs_iter_snapshots(zhp, B_FALSE, promote_snap_done_cb,
+                   &pd);
        }
 
        zfs_close(pzhp);

diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c
index 7a95de0755d6ff071696b7f6097cb4afbb9f591b..bc623733d5466912ed4ef2d8660060e607819478 100644 (file)
--- a/lib/libzfs/libzfs_sendrecv.c
+++ b/lib/libzfs/libzfs_sendrecv.c
@@ -21,6 +21,8 @@
 
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012 Pawel Jakub Dawidek <pawel@dawidek.net>.
+ * All rights reserved
  */
 
 #include <assert.h>
@@ -717,7 +719,7 @@ send_iterate_fs(zfs_handle_t *zhp, void *arg)
        sd->parent_fromsnap_guid = 0;
        VERIFY(0 == nvlist_alloc(&sd->parent_snaps, NV_UNIQUE_NAME, 0));
        VERIFY(0 == nvlist_alloc(&sd->snapprops, NV_UNIQUE_NAME, 0));
-       (void) zfs_iter_snapshots(zhp, send_iterate_snap, sd);
+       (void) zfs_iter_snapshots(zhp, B_FALSE, send_iterate_snap, sd);
        VERIFY(0 == nvlist_add_nvlist(nvfs, "snaps", sd->parent_snaps));
        VERIFY(0 == nvlist_add_nvlist(nvfs, "snapprops", sd->snapprops));
        nvlist_free(sd->parent_snaps);
@@ -843,7 +845,7 @@ zfs_iter_snapshots_sorted(zfs_handle_t *zhp, zfs_iter_f callback, void *data)
        avl_create(&avl, zfs_snapshot_compare,
            sizeof (zfs_node_t), offsetof(zfs_node_t, zn_avlnode));
 
-       ret = zfs_iter_snapshots(zhp, zfs_sort_snaps, &avl);
+       ret = zfs_iter_snapshots(zhp, B_FALSE, zfs_sort_snaps, &avl);
 
        for (node = avl_first(&avl); node != NULL; node = AVL_NEXT(&avl, node))
                ret |= callback(node->zn_handle, data);

diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c
index 17dc35d74cf0466b6960c923319b6384f2b74072..a232ed0d580b6a7f1f0646650864b4d7585dcd84 100644 (file)
--- a/module/zfs/zfs_ioctl.c
+++ b/module/zfs/zfs_ioctl.c
@@ -21,6 +21,7 @@
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
  * Portions Copyright 2011 Martin Matuska
+ * Portions Copyright 2012 Pawel Jakub Dawidek <pawel@dawidek.net>
  * Copyright (c) 2012, Joyent, Inc. All rights reserved.
  */
 
@@ -1973,7 +1974,7 @@ zfs_ioc_snapshot_list_next(zfs_cmd_t *zc)
        int error;
 
 top:
-       if (zc->zc_cookie == 0)
+       if (zc->zc_cookie == 0 && !zc->zc_simple)
                (void) dmu_objset_find(zc->zc_name, dmu_objset_prefetch,
                    NULL, DS_FIND_SNAPSHOTS);
 
@@ -1995,7 +1996,7 @@ top:
            zc->zc_name + strlen(zc->zc_name), &zc->zc_obj, &zc->zc_cookie,
            NULL);
 
-       if (error == 0) {
+       if (error == 0 && !zc->zc_simple) {
                dsl_dataset_t *ds;
                dsl_pool_t *dp = os->os_dsl_dataset->ds_dir->dd_pool;