From 0cee24064a79f9c01fc4521543c37acea538405f Mon Sep 17 00:00:00 2001 From: Pawel Jakub Dawidek Date: Tue, 29 May 2012 10:50:50 -0700 Subject: [PATCH] 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 Issue #450 --- cmd/zfs/zfs_iter.c | 30 +++++++++++++++++++++++++++--- cmd/zfs/zfs_iter.h | 2 ++ cmd/zfs/zfs_main.c | 14 +++++++++++++- include/libzfs.h | 2 +- include/sys/zfs_ioctl.h | 3 ++- lib/libzfs/libzfs_dataset.c | 41 +++++++++++++++++++++++++++++++++-------- lib/libzfs/libzfs_sendrecv.c | 6 ++++-- module/zfs/zfs_ioctl.c | 5 +++-- 8 files changed, 85 insertions(+), 18 deletions(-) diff --git a/cmd/zfs/zfs_iter.c b/cmd/zfs/zfs_iter.c index e2ab90e..6239a8f 100644 --- 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 . */ #include @@ -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 8c6b9fd..7f740e7 100644 --- 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 e20d570..0f00688 100644 --- 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 = "-"; @@ -2811,6 +2816,13 @@ zfs_do_list(int argc, char **argv) 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. */ if (strcmp(fields, "space") == 0 && types_specified == B_FALSE) diff --git a/include/libzfs.h b/include/libzfs.h index 26b1ce3..86e23ac 100644 --- 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 e3fd2c3..4e5c5fb 100644 --- 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 5434374..3340c71 100644 --- 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 . */ #include @@ -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 7a95de0..bc62373 100644 --- 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 . + * All rights reserved */ #include @@ -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 17dc35d..a232ed0 100644 --- 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 * 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; -- 1.8.3.1