[nbdkit PATCH] data, memory: Optimize .zero > PAGE_SIZE
When sparse_array_zero() is used for a range larger than a page, there's no need to waste time in memset() or is_zero() - we already know the page will be free()d. Signed-off-by: Eric Blake <eblake@redhat.com> --- Here's a fun one :) common/sparse/sparse.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/common/sparse/sparse.c b/common/sparse/sparse.c index cb44743c..5e085763 100644 --- a/common/sparse/sparse.c +++ b/common/sparse/sparse.c @@ -343,10 +343,13 @@ sparse_array_zero (struct sparse_array *sa, uint32_t count, uint64_t offset) n = count; if (p) { - memset (p, 0, n); + if (n < PAGE_SIZE) + memset (p, 0, n); + else + assert (p == *l2_page); /* If the whole page is now zero, free it. */ - if (is_zero (*l2_page, PAGE_SIZE)) { + if (n == PAGE_SIZE || is_zero (*l2_page, PAGE_SIZE)) { if (sa->debug) nbdkit_debug ("%s: freeing zero page at offset %" PRIu64, __func__, offset); -- 2.20.1
On Wed, Aug 14, 2019 at 09:10:15PM -0500, Eric Blake wrote:
When sparse_array_zero() is used for a range larger than a page, there's no need to waste time in memset() or is_zero() - we already know the page will be free()d.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
Here's a fun one :)
common/sparse/sparse.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/common/sparse/sparse.c b/common/sparse/sparse.c index cb44743c..5e085763 100644 --- a/common/sparse/sparse.c +++ b/common/sparse/sparse.c @@ -343,10 +343,13 @@ sparse_array_zero (struct sparse_array *sa, uint32_t count, uint64_t offset) n = count;
if (p) { - memset (p, 0, n); + if (n < PAGE_SIZE) + memset (p, 0, n); + else + assert (p == *l2_page);
/* If the whole page is now zero, free it. */ - if (is_zero (*l2_page, PAGE_SIZE)) { + if (n == PAGE_SIZE || is_zero (*l2_page, PAGE_SIZE)) {
Should this be n >= PAGE_SIZE? My guess is no because lookup (..&n..) can't return n larger than the remaining size of the page (ie. n <= PAGE_SIZE).
if (sa->debug) nbdkit_debug ("%s: freeing zero page at offset %" PRIu64, __func__, offset);
Anyway, ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
On 8/15/19 5:25 AM, Richard W.M. Jones wrote:
On Wed, Aug 14, 2019 at 09:10:15PM -0500, Eric Blake wrote:
When sparse_array_zero() is used for a range larger than a page, there's no need to waste time in memset() or is_zero() - we already know the page will be free()d.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
Here's a fun one :)
common/sparse/sparse.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/common/sparse/sparse.c b/common/sparse/sparse.c index cb44743c..5e085763 100644 --- a/common/sparse/sparse.c +++ b/common/sparse/sparse.c @@ -343,10 +343,13 @@ sparse_array_zero (struct sparse_array *sa, uint32_t count, uint64_t offset) n = count;
if (p) { - memset (p, 0, n); + if (n < PAGE_SIZE) + memset (p, 0, n); + else + assert (p == *l2_page);
/* If the whole page is now zero, free it. */ - if (is_zero (*l2_page, PAGE_SIZE)) { + if (n == PAGE_SIZE || is_zero (*l2_page, PAGE_SIZE)) {
Should this be n >= PAGE_SIZE? My guess is no because lookup (..&n..) can't return n larger than the remaining size of the page (ie. n <= PAGE_SIZE).
Correct, but that's action at a distance; using >= doesn't change semantics and brings the action closer to the code. I'll make that tweak.
if (sa->debug) nbdkit_debug ("%s: freeing zero page at offset %" PRIu64, __func__, offset);
Anyway, ACK.
Rich.
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
participants (2)
-
Eric Blake -
Richard W.M. Jones