Possible bug when setting compression

Possible bug when setting compression

[Matthew Eaton]

I have been testing iops performance using different compression
settings and found that setting either zero_buffers or
buffer_compress_percentage was not enough, I had to also set
scramble_buffers=0 in order to get the desired compression.  But this
was not the case when setting refill_buffers.  I tested with fio 2.1.5
and fio from git (fio-2.1.6.1-10-g4e59d).

My job file:

[global]
bs=4k
ioengine=libaio
iodepth=32
direct=1
size=8g
time_based
filename=/dev/sdf

#scramble_buffers=0
#refill_buffers
zero_buffers
#buffer_compress_percentage=100

[rand-write-iops]
rw=randwrite
runtime=10
stonewall

My results:

scramble_buffers=0
buffer_compress_percentage=100
iops=83254

scramble_buffers=0
buffer_compress_percentage=0
iops=51137

scramble_buffers=1
buffer_compress_percentage=100
iops=69639

scramble_buffers=1
buffer_compress_percentage=0
iops=69429

-----

scramble_buffers=0
zero_buffers
iops=83339

scramble_buffers=1
zero_buffers
iops=69510

-----

scramble_buffers=0
refill_buffers
iops=50774

scramble_buffers=1
refill_buffers
iops=50813

Re: Possible bug when setting compression

[Jens Axboe]

On 03/14/2014 09:52 AM, Matthew Eaton wrote:
> I have been testing iops performance using different compression
> settings and found that setting either zero_buffers or
> buffer_compress_percentage was not enough, I had to also set
> scramble_buffers=0 in order to get the desired compression.  But this
> was not the case when setting refill_buffers.  I tested with fio 2.1.5
> and fio from git (fio-2.1.6.1-10-g4e59d).

Interesting, it would make sense to simply not scramble if the 
compression settings are set. That is what happens with refill_buffers 
right now, it has precedence over the scrambling. Or if you give a 
buffer pattern, that also disables the scrambling. I'll add the same for 
the compression settings.

http://git.kernel.dk/?p=fio.git;a=commit;h=0574c885b82aea0332ab5fa35af84db0f3946726

Care to give it a test spin, just to be on the safe side?

-- 
Jens Axboe

Re: Possible bug when setting compression

[Matthew Eaton]

> Interesting, it would make sense to simply not scramble if the compression
> settings are set. That is what happens with refill_buffers right now, it has
> precedence over the scrambling. Or if you give a buffer pattern, that also
> disables the scrambling. I'll add the same for the compression settings.
>
> http://git.kernel.dk/?p=fio.git;a=commit;h=0574c885b82aea0332ab5fa35af84db0f3946726
>
> Care to give it a test spin, just to be on the safe side?
>
> --
> Jens Axboe
>

Hey Jens,

According to my test results below it worked for
buffer_compress_percentage, but not zero_buffers.  Also, it looks like
buffer_compress_percentage=0 is broken, unless =0 means disabling that
option altogether.

fio-2.1.6.1-11-g0574

buffer_compress_percentage=100
iops=83436

buffer_compress_percentage=50
iops=78625

buffer_compress_percentage=1
iops=52751

buffer_compress_percentage=0
iops=69433

zero_buffers
iops=69470

scramble_buffers=0
zero_buffers
iops=83369

Re: Possible bug when setting compression

[Jens Axboe]

[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]

On 03/14/2014 01:45 PM, Matthew Eaton wrote:
>> Interesting, it would make sense to simply not scramble if the compression
>> settings are set. That is what happens with refill_buffers right now, it has
>> precedence over the scrambling. Or if you give a buffer pattern, that also
>> disables the scrambling. I'll add the same for the compression settings.
>>
>> http://git.kernel.dk/?p=fio.git;a=commit;h=0574c885b82aea0332ab5fa35af84db0f3946726
>>
>> Care to give it a test spin, just to be on the safe side?
>>
>> --
>> Jens Axboe
>>
>
> Hey Jens,
>
> According to my test results below it worked for
> buffer_compress_percentage, but not zero_buffers.  Also, it looks like
> buffer_compress_percentage=0 is broken, unless =0 means disabling that
> option altogether.
>
> fio-2.1.6.1-11-g0574
>
> buffer_compress_percentage=100
> iops=83436
>
> buffer_compress_percentage=50
> iops=78625
>
> buffer_compress_percentage=1
> iops=52751
>
> buffer_compress_percentage=0
> iops=69433
>
> zero_buffers
> iops=69470
>
> scramble_buffers=0
> zero_buffers
> iops=83369
>

Doh yes, we can't use that as an enabled flag. Can you apply this on top 
of current git and see if it works? Untested...

-- 
Jens Axboe


[-- Attachment #2: fio-compress.patch --]
[-- Type: text/x-patch, Size: 2350 bytes --]

diff --git a/fio.h b/fio.h
index 52f1def7a28d..befdce31ee20 100644
--- a/fio.h
+++ b/fio.h
@@ -71,6 +71,7 @@ enum {
 	TD_F_SCRAMBLE_BUFFERS	= 16,
 	TD_F_VER_NONE		= 32,
 	TD_F_PROFILE_OPS	= 64,
+	TD_F_COMPRESS		= 128,
 };
 
 enum {
diff --git a/init.c b/init.c
index 0a872c0f1eed..73ec9eb22a8d 100644
--- a/init.c
+++ b/init.c
@@ -856,11 +856,6 @@ int ioengine_load(struct thread_data *td)
 	return 0;
 }
 
-static int compression_enabled(struct thread_options *o)
-{
-	return o->compress_percentage || o->compress_chunk;
-}
-
 static void init_flags(struct thread_data *td)
 {
 	struct thread_options *o = &td->o;
@@ -873,14 +868,8 @@ static void init_flags(struct thread_data *td)
 		td->flags |= TD_F_READ_IOLOG;
 	if (o->refill_buffers)
 		td->flags |= TD_F_REFILL_BUFFERS;
-
-	/*
-	 * Don't scramble buffers if we set any of the compression
-	 * settings
-	 */
-	if (o->scramble_buffers && !compression_enabled(o))
+	if (o->scramble_buffers)
 		td->flags |= TD_F_SCRAMBLE_BUFFERS;
-
 	if (o->verify != VERIFY_NONE)
 		td->flags |= TD_F_VER_NONE;
 }
diff --git a/io_u.c b/io_u.c
index 0b86d9f3c281..14645aeed50c 100644
--- a/io_u.c
+++ b/io_u.c
@@ -1490,7 +1490,8 @@ struct io_u *get_io_u(struct thread_data *td)
 			if (td->flags & TD_F_REFILL_BUFFERS) {
 				io_u_fill_buffer(td, io_u,
 					io_u->xfer_buflen, io_u->xfer_buflen);
-			} else if (td->flags & TD_F_SCRAMBLE_BUFFERS)
+			} else if ((td->flags & TD_F_SCRAMBLE_BUFFERS) &&
+				   !(td->flags & TD_F_COMPRESS))
 				do_scramble = 1;
 			if (td->flags & TD_F_VER_NONE) {
 				populate_verify_io_u(td, io_u);
diff --git a/options.c b/options.c
index 4ff4c9b42a7b..63d6848370bd 100644
--- a/options.c
+++ b/options.c
@@ -1002,6 +1002,14 @@ static int str_buffer_pattern_cb(void *data, const char *input)
 	return ret;
 }
 
+static int str_buffer_compress_cb(void *data, long long *il)
+{
+	struct thread_data *td = data;
+
+	td->flags |= TD_F_COMPRESS;
+	return 0;
+}
+
 static int str_verify_pattern_cb(void *data, const char *input)
 {
 	struct thread_data *td = data;
@@ -3119,6 +3127,7 @@ struct fio_option fio_options[FIO_MAX_OPTS] = {
 		.lname	= "Buffer compression percentage",
 		.type	= FIO_OPT_INT,
 		.off1	= td_var_offset(compress_percentage),
+		.cb	= str_buffer_compress_cb,
 		.maxval	= 100,
 		.minval	= 0,
 		.help	= "How compressible the buffer is (approximately)",

Re: Possible bug when setting compression

[Jens Axboe]

[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]

On 03/14/2014 02:09 PM, Jens Axboe wrote:
> On 03/14/2014 01:45 PM, Matthew Eaton wrote:
>>> Interesting, it would make sense to simply not scramble if the
>>> compression
>>> settings are set. That is what happens with refill_buffers right now,
>>> it has
>>> precedence over the scrambling. Or if you give a buffer pattern, that
>>> also
>>> disables the scrambling. I'll add the same for the compression settings.
>>>
>>> http://git.kernel.dk/?p=fio.git;a=commit;h=0574c885b82aea0332ab5fa35af84db0f3946726
>>>
>>>
>>> Care to give it a test spin, just to be on the safe side?
>>>
>>> --
>>> Jens Axboe
>>>
>>
>> Hey Jens,
>>
>> According to my test results below it worked for
>> buffer_compress_percentage, but not zero_buffers.  Also, it looks like
>> buffer_compress_percentage=0 is broken, unless =0 means disabling that
>> option altogether.
>>
>> fio-2.1.6.1-11-g0574
>>
>> buffer_compress_percentage=100
>> iops=83436
>>
>> buffer_compress_percentage=50
>> iops=78625
>>
>> buffer_compress_percentage=1
>> iops=52751
>>
>> buffer_compress_percentage=0
>> iops=69433
>>
>> zero_buffers
>> iops=69470
>>
>> scramble_buffers=0
>> zero_buffers
>> iops=83369
>>
>
> Doh yes, we can't use that as an enabled flag. Can you apply this on top
> of current git and see if it works? Untested...

So that wont work as-is, since the parser will bypass the store when the 
callback is specified. It needs a one-liner, please try this updated one 
instead.

-- 
Jens Axboe


[-- Attachment #2: fio-compress-v2.patch --]
[-- Type: text/x-patch, Size: 2394 bytes --]

diff --git a/fio.h b/fio.h
index 52f1def7a28d..befdce31ee20 100644
--- a/fio.h
+++ b/fio.h
@@ -71,6 +71,7 @@ enum {
 	TD_F_SCRAMBLE_BUFFERS	= 16,
 	TD_F_VER_NONE		= 32,
 	TD_F_PROFILE_OPS	= 64,
+	TD_F_COMPRESS		= 128,
 };
 
 enum {
diff --git a/init.c b/init.c
index 0a872c0f1eed..73ec9eb22a8d 100644
--- a/init.c
+++ b/init.c
@@ -856,11 +856,6 @@ int ioengine_load(struct thread_data *td)
 	return 0;
 }
 
-static int compression_enabled(struct thread_options *o)
-{
-	return o->compress_percentage || o->compress_chunk;
-}
-
 static void init_flags(struct thread_data *td)
 {
 	struct thread_options *o = &td->o;
@@ -873,14 +868,8 @@ static void init_flags(struct thread_data *td)
 		td->flags |= TD_F_READ_IOLOG;
 	if (o->refill_buffers)
 		td->flags |= TD_F_REFILL_BUFFERS;
-
-	/*
-	 * Don't scramble buffers if we set any of the compression
-	 * settings
-	 */
-	if (o->scramble_buffers && !compression_enabled(o))
+	if (o->scramble_buffers)
 		td->flags |= TD_F_SCRAMBLE_BUFFERS;
-
 	if (o->verify != VERIFY_NONE)
 		td->flags |= TD_F_VER_NONE;
 }
diff --git a/io_u.c b/io_u.c
index 0b86d9f3c281..14645aeed50c 100644
--- a/io_u.c
+++ b/io_u.c
@@ -1490,7 +1490,8 @@ struct io_u *get_io_u(struct thread_data *td)
 			if (td->flags & TD_F_REFILL_BUFFERS) {
 				io_u_fill_buffer(td, io_u,
 					io_u->xfer_buflen, io_u->xfer_buflen);
-			} else if (td->flags & TD_F_SCRAMBLE_BUFFERS)
+			} else if ((td->flags & TD_F_SCRAMBLE_BUFFERS) &&
+				   !(td->flags & TD_F_COMPRESS))
 				do_scramble = 1;
 			if (td->flags & TD_F_VER_NONE) {
 				populate_verify_io_u(td, io_u);
diff --git a/options.c b/options.c
index 4ff4c9b42a7b..1ffb4b177d25 100644
--- a/options.c
+++ b/options.c
@@ -1002,6 +1002,15 @@ static int str_buffer_pattern_cb(void *data, const char *input)
 	return ret;
 }
 
+static int str_buffer_compress_cb(void *data, unsigned long long *il)
+{
+	struct thread_data *td = data;
+
+	td->flags |= TD_F_COMPRESS;
+	td->o.compress_percentage = *il;
+	return 0;
+}
+
 static int str_verify_pattern_cb(void *data, const char *input)
 {
 	struct thread_data *td = data;
@@ -3119,6 +3128,7 @@ struct fio_option fio_options[FIO_MAX_OPTS] = {
 		.lname	= "Buffer compression percentage",
 		.type	= FIO_OPT_INT,
 		.off1	= td_var_offset(compress_percentage),
+		.cb	= str_buffer_compress_cb,
 		.maxval	= 100,
 		.minval	= 0,
 		.help	= "How compressible the buffer is (approximately)",

Re: Possible bug when setting compression

[Matthew Eaton]

> So that wont work as-is, since the parser will bypass the store when the
> callback is specified. It needs a one-liner, please try this updated one
> instead.
>
> --
> Jens Axboe
>

Ok, so it looks like everything works now except for zero_buffers.

fio-2.1.6.1-11-g0574 + patch

buffer_compress_percentage=100
iops=83457

buffer_compress_percentage=50
iops=78836

buffer_compress_percentage=1
iops=52785

buffer_compress_percentage=0
iops=51013

zero_buffers
iops=69465

scramble_buffers=0
zero_buffers
iops=83451

Re: Possible bug when setting compression

[Jens Axboe]

On 03/14/2014 02:53 PM, Matthew Eaton wrote:
>> So that wont work as-is, since the parser will bypass the store when the
>> callback is specified. It needs a one-liner, please try this updated one
>> instead.
>>
>> --
>> Jens Axboe
>>
>
> Ok, so it looks like everything works now except for zero_buffers.
>
> fio-2.1.6.1-11-g0574 + patch
>
> buffer_compress_percentage=100
> iops=83457
>
> buffer_compress_percentage=50
> iops=78836
>
> buffer_compress_percentage=1
> iops=52785
>
> buffer_compress_percentage=0
> iops=51013
>
> zero_buffers
> iops=69465
>
> scramble_buffers=0
> zero_buffers
> iops=83451

zero_buffers and buffer_compres_percentage=100 are essentially the same 
thing, for both cases it writes buffers just fill of zeroes. So I'm not 
sure what you were expecting here since you don't think it works as it 
should?

If you have both zero_buffers and scramble set, then the result will be 
a bit less compressible. The goal of scramble_buffers is to defeat basic 
dedupe attempts. So even if you don't fully refill your buffers every 
time, the blocks will be a bit different.

-- 
Jens Axboe

Re: Possible bug when setting compression

[Matthew Eaton]

> zero_buffers and buffer_compres_percentage=100 are essentially the same
> thing, for both cases it writes buffers just fill of zeroes. So I'm not sure
> what you were expecting here since you don't think it works as it should?
>
> If you have both zero_buffers and scramble set, then the result will be a
> bit less compressible. The goal of scramble_buffers is to defeat basic
> dedupe attempts. So even if you don't fully refill your buffers every time,
> the blocks will be a bit different.
>
> --
> Jens Axboe
>
>

Hmm, I was assuming that scramble_buffers shouldn't have any effect
since it's scrambling all zeros.  Is scramble_buffers inserting random
data even with zero_buffers set?

Re: Possible bug when setting compression

[Jens Axboe]

On 03/14/2014 03:06 PM, Matthew Eaton wrote:
>> zero_buffers and buffer_compres_percentage=100 are essentially the same
>> thing, for both cases it writes buffers just fill of zeroes. So I'm not sure
>> what you were expecting here since you don't think it works as it should?
>>
>> If you have both zero_buffers and scramble set, then the result will be a
>> bit less compressible. The goal of scramble_buffers is to defeat basic
>> dedupe attempts. So even if you don't fully refill your buffers every time,
>> the blocks will be a bit different.
>>
>> --
>> Jens Axboe
>>
>>
>
> Hmm, I was assuming that scramble_buffers shouldn't have any effect
> since it's scrambling all zeros.  Is scramble_buffers inserting random
> data even with zero_buffers set?

scramble_buffers tells fio to randomly insert some data in the buffer. 
It's not a full overwrite or anything like that, just some basic garbage 
for each 512b to prevent naive compression/dedupe.

-- 
Jens Axboe

Re: Possible bug when setting compression

[Matthew Eaton]

> scramble_buffers tells fio to randomly insert some data in the buffer. It's
> not a full overwrite or anything like that, just some basic garbage for each
> 512b to prevent naive compression/dedupe.
>
> --
> Jens Axboe
>
>

Thanks, that makes sense.  I guess my only concern is one might expect
that setting zero_buffers alone would provide fully compressible data,
not realizing that scramble_buffers is still adding some random data
unless explicitly turned off.

Maybe something that could be added to the man page / how to under
zero_buffers?  Something like...

"If you want a fully compressible workload, also set scramble_buffers=0."

Re: Possible bug when setting compression

[Jens Axboe]

On 2014-03-14 16:26, Matthew Eaton wrote:
>> scramble_buffers tells fio to randomly insert some data in the buffer. It's
>> not a full overwrite or anything like that, just some basic garbage for each
>> 512b to prevent naive compression/dedupe.
>>
>> --
>> Jens Axboe
>>
>>
>
> Thanks, that makes sense.  I guess my only concern is one might expect
> that setting zero_buffers alone would provide fully compressible data,
> not realizing that scramble_buffers is still adding some random data
> unless explicitly turned off.
>
> Maybe something that could be added to the man page / how to under
> zero_buffers?  Something like...
>
> "If you want a fully compressible workload, also set scramble_buffers=0."

Sure, we can add that to make it completely clear.

-- 
Jens Axboe