Skip checksum benchmarks on systems with slow cpu #13695

mcmilk · 2022-07-26T08:33:56Z

The checksum benchmarking on module load may take a really long time
on embedded systems with a slow cpu. Avoid all benchmarks >= 1MiB on
systems, where EdonR is slower then 300 MiB/s.

This limit is currently hardcoded, but could also be a tuneable.
When we switch to a tuneable, what name should I use then?

This is the new benchmark output of a slow Intel Atom D510:

 implementation    1k    4k   16k   64k  256k
 edonr-generic    209   257   268   259   262
 skein-generic    129   150   151   150   150
 sha256-generic    50    55    56    56    56
 sha512-generic    76    86    88    89    88
 blake3-generic    63    62    62    62    61
 blake3-sse2      114   292   301   307   309

Signed-off-by: Tino Reichardt milky-zfs@mcmilk.de

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the OpenZFS code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
I have run the ZFS Test Suite with this change applied.
All commit messages are properly formatted and contain Signed-off-by.

mcmilk · 2022-07-28T18:36:35Z

I have no idea why the FreeBSD and CentOS tests are failing currently ... on my local BSD it's working fine.
Maybe the CI has some full disk?

Can I remove the following comment from the git commit message?

This limit is currently hardcoded, but could also be a tuneable.
When we switch to a tuneable, what name should I use then?

I could also make it a #define in the beginning of zfs_chksum.c.

behlendorf · 2022-07-28T21:29:29Z

module/zfs/zfs_chksum.c

+
+	/* check if we ran on a slow cpu */
+	if (limit == LIMIT_INIT) {
+		if (cs->bs1k < 300) {


I think leaving this hard coded is fine, but let's at least add a #define to specify it for clarity. Maybe:

/* #define LIMIT_PERF_MBS 300

/* limit benchmarking to max 256KiB, when EdonR is slower then this: */ #define LIMIT_PERF_MBS 300

module/zfs/zfs_chksum.c

behlendorf · 2022-07-28T21:38:53Z

module/zfs/zfs_chksum.c

+		}
+	}
+
+	/* skip benchmarks >= 1MiB, when edonr is slower then 300 MiB */


What makes this specific to edonr? It seems like this is only mostly true since edonr gets benchmarked first. If that were to change then this would no longer be accurate.

Yes, that is correct, I will add a comment that edonr needs to be the first one.

behlendorf · 2022-07-28T21:49:06Z

module/zfs/zfs_chksum.c

+#define	LIMIT_NEEDED	1
+#define	LIMIT_NOLIMIT	-1
+
+static int limit = LIMIT_INIT;


Suggested change

static int limit = LIMIT_INIT;

static int chksum_stat_limit = LIMIT_INIT;

Or perhaps better yet you could avoid the global variable, move this in to chksum_benchmark(), and then add a flag to chksum_benchit() to request either the mini or full test run. You could still early abort a full test run if any test too longer than some limit.

I had it first within the function, but then the header stuff looked not so nice, so I decided to use two different header functions. But the 3x 0's and one headline are better for parsing - thats a point - I will switch back to my first version here.

behlendorf · 2022-07-28T21:59:50Z

module/zfs/zfs_chksum.c

+	(void) snprintf(buf + off, size - off, "%8s", "256k\n");
+
+	return (0);
+}


Good thought, although always printing the full header with 0's for tests which weren't run would make this a little easier to parse.

The checksum benchmarking on module load may take a really long time on embedded systems with a slow cpu. Avoid all benchmarks >= 1MiB on systems, where EdonR is slower then 300 MiB/s. This limit is currently hardcoded via the define LIMIT_PERF_MBS. This is the new benchmark output of a slow Intel Atom: ``` implementation 1k 4k 16k 64k 256k 1m 4m 16m edonr-generic 209 257 268 259 262 0 0 0 skein-generic 129 150 151 150 150 0 0 0 sha256-generic 50 55 56 56 56 0 0 0 sha512-generic 76 86 88 89 88 0 0 0 blake3-generic 63 62 62 62 61 0 0 0 blake3-sse2 114 292 301 307 309 0 0 0 ``` Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de>

mcmilk · 2022-07-28T22:43:13Z

It looks like this now:

SLOW CPU:
[mcmilk@freebsd ~/bin]$ sysctl kstat.zfs.misc.chksum_bench
kstat.zfs.misc.chksum_bench: 
implementation               1k      4k     16k     64k    256k      1m      4m     16m
edonr-generic              *<300*  2362    2488    2509    2511       0       0       0
skein-generic               312     337     343     343     345       0       0       0
sha256-generic              401     428     435     439     439       0       0       0
sha512-generic              585     663     692     702     699       0       0       0
blake3-generic              615     602     584     592     598       0       0       0
blake3-sse2                 309    1935    2129    2192    2180       0       0       0
blake3-sse41                324    2154    2370    2455    2407       0       0       0
blake3-avx2                 323    2668    4345    4825    4741       0       0       0

[mcmilk@freebsd ~/bin]$ sysctl kstat.zfs.misc.chksum_bench
kstat.zfs.misc.chksum_bench: 
implementation               1k      4k     16k     64k    256k      1m      4m     16m
edonr-generic              1985    2362    2419    2493    2482    2489    2264    2339
skein-generic               309     334     341     342     342     343     342     341
sha256-generic              399     426     436     438     436     439     436     434
sha512-generic              587     667     689     694     697     698     689     688
blake3-generic              610     601     581     598     597     598     590     589
blake3-sse2                 309    1919    2113    2179    2165    2140    1749    1737
blake3-sse41                321    2151    2368    2459    2443    2413    1947    1925
blake3-avx2                 325    2676    4332    4713    4741    4586    2254    2281

Should be ready.

The checksum benchmarking on module load may take a really long time on embedded systems with a slow cpu. Avoid all benchmarks >= 1MiB on systems, where EdonR is slower then 300 MiB/s. This limit is currently hardcoded via the define LIMIT_PERF_MBS. This is the new benchmark output of a slow Intel Atom: ``` implementation 1k 4k 16k 64k 256k 1m 4m 16m edonr-generic 209 257 268 259 262 0 0 0 skein-generic 129 150 151 150 150 0 0 0 sha256-generic 50 55 56 56 56 0 0 0 sha512-generic 76 86 88 89 88 0 0 0 blake3-generic 63 62 62 62 61 0 0 0 blake3-sse2 114 292 301 307 309 0 0 0 ``` Reviewed-by: Sebastian Gottschall <s.gottschall@dd-wrt.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Closes openzfs#13695

mcmilk force-pushed the blake3-limit-benchmarks branch 7 times, most recently from 0bd14d9 to b122ea6 Compare July 26, 2022 11:26

behlendorf added the Status: Code Review Needed Ready for review and testing label Jul 26, 2022

BrainSlayer approved these changes Jul 28, 2022

View reviewed changes

behlendorf reviewed Jul 28, 2022

View reviewed changes

mcmilk force-pushed the blake3-limit-benchmarks branch 2 times, most recently from 7cd34f3 to 6423b67 Compare July 28, 2022 22:33

mcmilk force-pushed the blake3-limit-benchmarks branch from 6423b67 to fe7e15f Compare July 28, 2022 22:39

behlendorf approved these changes Jul 28, 2022

View reviewed changes

behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jul 28, 2022

behlendorf merged commit 68aa337 into openzfs:master Aug 1, 2022

mcmilk deleted the blake3-limit-benchmarks branch August 3, 2022 07:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Skip checksum benchmarks on systems with slow cpu #13695

Skip checksum benchmarks on systems with slow cpu #13695

mcmilk commented Jul 26, 2022 •

edited

Loading

mcmilk commented Jul 28, 2022

behlendorf Jul 28, 2022

mcmilk Jul 28, 2022

behlendorf Jul 28, 2022

mcmilk Jul 28, 2022

behlendorf Jul 28, 2022

mcmilk Jul 28, 2022

behlendorf Jul 28, 2022

mcmilk commented Jul 28, 2022 •

edited

Loading

	static int limit = LIMIT_INIT;
	static int chksum_stat_limit = LIMIT_INIT;

Skip checksum benchmarks on systems with slow cpu #13695

Skip checksum benchmarks on systems with slow cpu #13695

Conversation

mcmilk commented Jul 26, 2022 • edited Loading

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

mcmilk commented Jul 28, 2022

behlendorf Jul 28, 2022

Choose a reason for hiding this comment

mcmilk Jul 28, 2022

Choose a reason for hiding this comment

behlendorf Jul 28, 2022

Choose a reason for hiding this comment

mcmilk Jul 28, 2022

Choose a reason for hiding this comment

behlendorf Jul 28, 2022

Choose a reason for hiding this comment

mcmilk Jul 28, 2022

Choose a reason for hiding this comment

behlendorf Jul 28, 2022

Choose a reason for hiding this comment

mcmilk commented Jul 28, 2022 • edited Loading

mcmilk commented Jul 26, 2022 •

edited

Loading

mcmilk commented Jul 28, 2022 •

edited

Loading