Re: [PATCH bpf-next v8] selftests/bpf: Add benchmark for bpf memory allocator

From: Hou Tao <houtao@huaweicloud.com>
To: John Fastabend <john.fastabend@gmail.com>,
	bpf@vger.kernel.org,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Andrii Nakryiko <andrii@kernel.org>, Song Liu <song@kernel.org>,
	Hao Luo <haoluo@google.com>, Yonghong Song <yhs@fb.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@google.com>, Jiri Olsa <jolsa@kernel.org>,
	"Paul E . McKenney" <paulmck@kernel.org>,
	rcu@vger.kernel.org, houtao1@huawei.com,
	Martin KaFai Lau <martin.lau@linux.dev>
Subject: Re: [PATCH bpf-next v8] selftests/bpf: Add benchmark for bpf memory allocator
Date: Tue, 4 Jul 2023 09:13:52 +0800	[thread overview]
Message-ID: <47eb0f85-5950-bb15-079f-383a865e27c0@huaweicloud.com> (raw)
In-Reply-To: <64a34309e42aa_652052084f@john.notmuch>

Hi,

On 7/4/2023 5:52 AM, John Fastabend wrote:
> Hou Tao wrote:
>> From: Hou Tao <houtao1@huawei.com>
>>
>> The benchmark could be used to compare the performance of hash map
>> operations and the memory usage between different flavors of bpf memory
>> allocator (e.g., no bpf ma vs bpf ma vs reuse-after-gp bpf ma). It also
>> could be used to check the performance improvement or the memory saving
>> provided by optimization.
>>

SNIP
>> overwrite            per-prod-op: 191.92 ± 0.07k/s, avg mem: 1.23 ± 0.00MiB, peak mem: 1.49MiB
>> batch_add_batch_del  per-prod-op: 218.10 ± 0.25k/s, avg mem: 1.23 ± 0.00MiB, peak mem: 1.49MiB
>> add_del_on_diff_cpu  per-prod-op: 39.59 ± 0.41k/s, avg mem: 1.48 ± 0.11MiB, peak mem: 1.74MiB
>>
>> (3) normal bpf memory allocator
>>
>> overwrite            per-prod-op: 134.81 ± 0.22k/s, avg mem: 1.67 ± 0.12MiB, peak mem: 2.74MiB
>> batch_add_batch_del  per-prod-op: 90.44 ± 0.34k/s, avg mem: 2.27 ± 0.00MiB, peak mem: 2.74MiB
>> add_del_on_diff_cpu  per-prod-op: 28.20 ± 0.15k/s, avg mem: 1.73 ± 0.17MiB, peak mem: 2.06MiB
> Acked-by: John Fastabend <john.fastabend@gmail.com>
Thanks for the Ack.
>
>> +
>> +static error_t htab_mem_parse_arg(int key, char *arg, struct argp_state *state)
>> +{
>> +	switch (key) {
>> +	case ARG_VALUE_SIZE:
>> +		args.value_size = strtoul(arg, NULL, 10);
>> +		if (args.value_size > 4096) {
>> +			fprintf(stderr, "too big value size %u\n", args.value_size);
>> +			argp_usage(state);
>> +		}
>> +		break;
>> +	case ARG_USE_CASE:
>> +		args.use_case = strdup(arg);
> might be worth checking for null and returning an error? Only matters if we
> run from CI or something and then this looks like a flake.
Will fix. I also found an error reported from BPF CI llvm-16 build
procedure: "benchs/bench_htab_mem.c:45:27: error: initializer element is
not a compile-time constant". Will fix in v9.
>
>> +		break;
>> +	case ARG_PREALLOCATED:
>> +		args.preallocated = true;
>> +		break;
>> +	default:
>> +		return ARGP_ERR_UNKNOWN;
>> +	}
>> +
>> +	return 0;
>> +}