Re: [PATCH 3/4] populate: improve attr creation runtime

[Zorro Lang <zlang@redhat.com>]

On Tue, Jan 17, 2023 at 04:44:18PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Replace the file creation loops with a python script that does
> everything we want from a single process.  This reduces the runtime of
> _scratch_xfs_populate substantially by avoiding thousands of execve
> overhead.  This patch builds on the previous one by reducing the runtime
> of xfs/349 from ~45s to ~15s.
> 
> For people who don't have python3, use setfattr's "restore" mode to bulk
> create xattrs.  This reduces runtime to about ~25s.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Thanks for making a fallback if there's not python3. The python logic and
that fallback logic all look good to me.

Reviewed-by: Zorro Lang <zlang@redhat.com>

Thanks,
Zorro

>  common/populate |   22 +++++++++++++++++---
>  src/popattr.py  |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 81 insertions(+), 3 deletions(-)
>  create mode 100755 src/popattr.py
> 
> 
> diff --git a/common/populate b/common/populate
> index 180540aedd..f34551d272 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -12,6 +12,10 @@ _require_populate_commands() {
>  	_require_xfs_io_command "fpunch"
>  	_require_test_program "punch-alternating"
>  	_require_test_program "popdir.pl"
> +	if [ -n "${PYTHON3_PROG}" ]; then
> +		_require_command $PYTHON3_PROG python3
> +		_require_test_program "popattr.py"
> +	fi
>  	case "${FSTYP}" in
>  	"xfs")
>  		_require_command "$XFS_DB_PROG" "xfs_db"
> @@ -108,9 +112,21 @@ __populate_create_attr() {
>  	missing="$3"
>  
>  	touch "${name}"
> -	seq 0 "${nr}" | while read d; do
> -		setfattr -n "user.$(printf "%.08d" "$d")" -v "$(printf "%.08d" "$d")" "${name}"
> -	done
> +
> +	if [ -n "${PYTHON3_PROG}" ]; then
> +		${PYTHON3_PROG} $here/src/popattr.py --file "${name}" --end "${nr}"
> +
> +		test -z "${missing}" && return
> +		${PYTHON3_PROG} $here/src/popattr.py --file "${name}" --start 1 --incr 2 --end "${nr}" --remove
> +		return
> +	fi
> +
> +	# Simulate a getfattr dump file so we can bulk-add attrs.
> +	(
> +		echo "# file: ${name}";
> +		seq --format "user.%08g=\"abcdefgh\"" 0 "${nr}"
> +		echo
> +	) | setfattr --restore -
>  
>  	test -z "${missing}" && return
>  	seq 1 2 "${nr}" | while read d; do
> diff --git a/src/popattr.py b/src/popattr.py
> new file mode 100755
> index 0000000000..397ced9d33
> --- /dev/null
> +++ b/src/popattr.py
> @@ -0,0 +1,62 @@
> +#!/usr/bin/python3
> +
> +# Copyright (c) 2023 Oracle.  All rights reserved.
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Create a bunch of xattrs in a file.
> +
> +import argparse
> +import sys
> +import os
> +
> +parser = argparse.ArgumentParser(description = 'Mass create xattrs in a file')
> +parser.add_argument(
> +	'--file', required = True, type = str, help = 'manipulate this file')
> +parser.add_argument(
> +	'--start', type = int, default = 0,
> +	help = 'create xattrs starting with this number')
> +parser.add_argument(
> +	'--incr', type = int, default = 1,
> +	help = 'increment attr number by this much')
> +parser.add_argument(
> +	'--end', type = int, default = 1000,
> +	help = 'stop at this attr number')
> +parser.add_argument(
> +	'--remove', dest = 'remove', action = 'store_true',
> +	help = 'remove instead of creating')
> +parser.add_argument(
> +	'--format', type = str, default = '%08d',
> +	help = 'printf formatting string for attr name')
> +parser.add_argument(
> +	'--verbose', dest = 'verbose', action = 'store_true',
> +	help = 'verbose output')
> +
> +args = parser.parse_args()
> +
> +fmtstring = "user.%s" % args.format
> +
> +# If we are passed a regular file, open it as a proper file descriptor and
> +# pass that around for speed.  Otherwise, we pass the path.
> +fp = None
> +try:
> +	fp = open(args.file, 'r')
> +	fd = fp.fileno()
> +	os.listxattr(fd)
> +	if args.verbose:
> +		print("using fd calls")
> +except:
> +	if args.verbose:
> +		print("using path calls")
> +	fd = args.file
> +
> +for i in range(args.start, args.end + 1, args.incr):
> +	fname = fmtstring % i
> +
> +	if args.remove:
> +		if args.verbose:
> +			print("removexattr %s" % fname)
> +		os.removexattr(fd, fname)
> +	else:
> +		if args.verbose:
> +			print("setxattr %s" % fname)
> +		os.setxattr(fd, fname, b'abcdefgh')
>